[PATCH v9 0/7] Define MOCS table for Icelake

[PATCH v9 0/7] Define MOCS table for Icelake

[Lucas De Marchi]

v9 of https://patchwork.freedesktop.org/series/54070/

Changes:
  - Add the R-b received
  - Wrap lines in "drm/i915: use a macro to define MOCS entries"
  - Add helper functions in
    "drm/i915: keep track of used entries in MOCS table"

Lucas De Marchi (5):
  drm/i915: initialize unused MOCS entries to PTE
  drm/i915: Simplify MOCS table definition
  drm/i915: use a macro to define MOCS entries
  drm/i915: keep track of used entries in MOCS table
  drm/i915: cache number of MOCS entries

Tomasz Lis (2):
  drm/i915/skl: Rework MOCS tables to keep common part in a define
  drm/i915/icl: Define MOCS table for Icelake

 drivers/gpu/drm/i915/intel_mocs.c | 406 +++++++++++++++++++-----------
 1 file changed, 260 insertions(+), 146 deletions(-)

-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 1/7] drm/i915: initialize unused MOCS entries to PTE

[Lucas De Marchi]

Instead of initializing them to uncached, let's set them to PTE for
kernel tracking. While at it do some minor adjustments to comments and
coding style.

From Chris: "What it does mean is that the buffer contents are consistent
with our cache tracking; and for userspace the results were always
undefined. So we should at least be able to guarantee that the data
written by userspace from the CPU is visible. After that, your caches
are on your own".

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 56 +++++++++++++------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index e976c5ce5479..0d6b94a239d6 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -85,10 +85,7 @@ struct drm_i915_mocs_table {
  *
  * Entries not part of the following tables are undefined as far as
  * userspace is concerned and shouldn't be relied upon.  For the time
- * being they will be implicitly initialized to the strictest caching
- * configuration (uncached) to guarantee forwards compatibility with
- * userspace programs written against more recent kernels providing
- * additional MOCS entries.
+ * being they will be initialized to PTE.
  *
  * NOTE: These tables MUST start with being uncached and the length
  *       MUST be less than 63 as the last two registers are reserved
@@ -249,16 +246,13 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 			   table.table[index].control_value);
 
 	/*
-	 * Ok, now set the unused entries to uncached. These entries
-	 * are officially undefined and no contract for the contents
-	 * and settings is given for these entries.
-	 *
-	 * Entry 0 in the table is uncached - so we are just writing
-	 * that value to all the used entries.
+	 * Now set the unused entries to PTE. These entries are officially
+	 * undefined and no contract for the contents and settings is given
+	 * for these entries.
 	 */
 	for (; index < GEN9_NUM_MOCS_ENTRIES; index++)
 		I915_WRITE(mocs_register(engine->id, index),
-			   table.table[0].control_value);
+			   table.table[I915_MOCS_PTE].control_value);
 }
 
 /**
@@ -293,16 +287,13 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	}
 
 	/*
-	 * Ok, now set the unused entries to uncached. These entries
-	 * are officially undefined and no contract for the contents
-	 * and settings is given for these entries.
-	 *
-	 * Entry 0 in the table is uncached - so we are just writing
-	 * that value to all the used entries.
+	 * Now set the unused entries to PTE. These entries are officially
+	 * undefined and no contract for the contents and settings is given
+	 * for these entries.
 	 */
 	for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
 		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
-		*cs++ = table->table[0].control_value;
+		*cs++ = table->table[I915_MOCS_PTE].control_value;
 	}
 
 	*cs++ = MI_NOOP;
@@ -345,7 +336,7 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 
 	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2);
 
-	for (i = 0; i < table->size/2; i++) {
+	for (i = 0; i < table->size / 2; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
 		*cs++ = l3cc_combine(table, 2 * i, 2 * i + 1);
 	}
@@ -353,18 +344,18 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 	if (table->size & 0x01) {
 		/* Odd table size - 1 left over */
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, 2 * i, 0);
+		*cs++ = l3cc_combine(table, 2 * i, I915_MOCS_PTE);
 		i++;
 	}
 
 	/*
-	 * Now set the rest of the table to uncached - use entry 0 as
-	 * this will be uncached. Leave the last pair uninitialised as
-	 * they are reserved by the hardware.
+	 * Now set the unused entries to PTE. These entries are officially
+	 * undefined and no contract for the contents and settings is given
+	 * for these entries.
 	 */
 	for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, 0, 0);
+		*cs++ = l3cc_combine(table, I915_MOCS_PTE, I915_MOCS_PTE);
 	}
 
 	*cs++ = MI_NOOP;
@@ -395,22 +386,21 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
 	if (!get_mocs_settings(dev_priv, &table))
 		return;
 
-	for (i = 0; i < table.size/2; i++)
-		I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 2*i, 2*i+1));
+	for (i = 0; i < table.size / 2; i++)
+		I915_WRITE(GEN9_LNCFCMOCS(i),
+			   l3cc_combine(&table, 2 * i, 2 * i + 1));
 
 	/* Odd table size - 1 left over */
 	if (table.size & 0x01) {
-		I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 2*i, 0));
+		I915_WRITE(GEN9_LNCFCMOCS(i),
+			   l3cc_combine(&table, 2 * i, I915_MOCS_PTE));
 		i++;
 	}
 
-	/*
-	 * Now set the rest of the table to uncached - use entry 0 as
-	 * this will be uncached. Leave the last pair as initialised as
-	 * they are reserved by the hardware.
-	 */
+	/* Now set the rest of the table to PTE */
 	for (; i < (GEN9_NUM_MOCS_ENTRIES / 2); i++)
-		I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 0, 0));
+		I915_WRITE(GEN9_LNCFCMOCS(i),
+			   l3cc_combine(&table, I915_MOCS_PTE, I915_MOCS_PTE));
 }
 
 /**
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 2/7] drm/i915: Simplify MOCS table definition

[Lucas De Marchi]

Make the defines for LE and L3 caching options to contain the shifts and
remove the zeros from the tables as shifting zeros always result in
zero.

Starting from Ice Lake the MOCS table is defined in the spec and
contains all entries. So to simplify checking the table with the values
set in code, the value is now part of the macro name. This allows to
still give the most used option and sensible name, but also to easily
cross check the table from the spec for gen >= 11.

By removing the zeros we avoid maintaining a huge table since the one
from spec contains many more entries. The new table for Ice Lake will
be added by other patches, this only reformats the table.

While at it also fix the indentation.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 80 +++++++++++--------------------
 1 file changed, 29 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 0d6b94a239d6..4ea80bb7dcc8 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -36,8 +36,8 @@ struct drm_i915_mocs_table {
 };
 
 /* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */
-#define LE_CACHEABILITY(value)	((value) << 0)
-#define LE_TGT_CACHE(value)	((value) << 2)
+#define _LE_CACHEABILITY(value)	((value) << 0)
+#define _LE_TGT_CACHE(value)	((value) << 2)
 #define LE_LRUM(value)		((value) << 4)
 #define LE_AOM(value)		((value) << 6)
 #define LE_RSC(value)		((value) << 7)
@@ -48,28 +48,28 @@ struct drm_i915_mocs_table {
 /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
 #define L3_ESC(value)		((value) << 0)
 #define L3_SCC(value)		((value) << 1)
-#define L3_CACHEABILITY(value)	((value) << 4)
+#define _L3_CACHEABILITY(value)	((value) << 4)
 
 /* Helper defines */
 #define GEN9_NUM_MOCS_ENTRIES	62  /* 62 out of 64 - 63 & 64 are reserved. */
 
 /* (e)LLC caching options */
-#define LE_PAGETABLE		0
-#define LE_UC			1
-#define LE_WT			2
-#define LE_WB			3
-
-/* L3 caching options */
-#define L3_DIRECT		0
-#define L3_UC			1
-#define L3_RESERVED		2
-#define L3_WB			3
+#define LE_0_PAGETABLE		_LE_CACHEABILITY(0)
+#define LE_1_UC			_LE_CACHEABILITY(1)
+#define LE_2_WT			_LE_CACHEABILITY(2)
+#define LE_3_WB			_LE_CACHEABILITY(3)
 
 /* Target cache */
-#define LE_TC_PAGETABLE		0
-#define LE_TC_LLC		1
-#define LE_TC_LLC_ELLC		2
-#define LE_TC_LLC_ELLC_ALT	3
+#define LE_TC_0_PAGETABLE	_LE_TGT_CACHE(0)
+#define LE_TC_1_LLC		_LE_TGT_CACHE(1)
+#define LE_TC_2_LLC_ELLC	_LE_TGT_CACHE(2)
+#define LE_TC_3_LLC_ELLC_ALT	_LE_TGT_CACHE(3)
+
+/* L3 caching options */
+#define L3_0_DIRECT		_L3_CACHEABILITY(0)
+#define L3_1_UC			_L3_CACHEABILITY(1)
+#define L3_2_RESERVED		_L3_CACHEABILITY(2)
+#define L3_3_WB			_L3_CACHEABILITY(3)
 
 /*
  * MOCS tables
@@ -96,31 +96,21 @@ struct drm_i915_mocs_table {
 static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
 	[I915_MOCS_UNCACHED] = {
 	  /* 0x00000009 */
-	  .control_value = LE_CACHEABILITY(LE_UC) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
+	  .control_value = LE_1_UC | LE_TC_2_LLC_ELLC,
 	  /* 0x0010 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
+	  .l3cc_value =    L3_1_UC,
 	},
 	[I915_MOCS_PTE] = {
 	  /* 0x00000038 */
-	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
+	  .control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3),
 	  /* 0x0030 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	  .l3cc_value =    L3_3_WB,
 	},
 	[I915_MOCS_CACHED] = {
 	  /* 0x0000003b */
-	  .control_value = LE_CACHEABILITY(LE_WB) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
+	  .control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
 	  /* 0x0030 */
-	  .l3cc_value =   L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	  .l3cc_value =   L3_3_WB,
 	},
 };
 
@@ -128,33 +118,21 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
 static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
 	[I915_MOCS_UNCACHED] = {
 	  /* 0x00000009 */
-	  .control_value = LE_CACHEABILITY(LE_UC) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
+	  .control_value = LE_1_UC | LE_TC_2_LLC_ELLC,
 	  /* 0x0010 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
+	  .l3cc_value = L3_1_UC,
 	},
 	[I915_MOCS_PTE] = {
 	  /* 0x00000038 */
-	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
+	  .control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3),
 	  /* 0x0030 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	  .l3cc_value = L3_3_WB,
 	},
 	[I915_MOCS_CACHED] = {
 	  /* 0x00000039 */
-	  .control_value = LE_CACHEABILITY(LE_UC) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
+	  .control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
 	  /* 0x0030 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	  .l3cc_value = L3_3_WB,
 	},
 };
 
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 3/7] drm/i915/skl: Rework MOCS tables to keep common part in a define

[Lucas De Marchi]

From: Tomasz Lis <tomasz.lis@intel.com>

The MOCS tables are going to be very similar across platforms.

To reduce the amount of copied code, this patch rips the common part and
puts it into a definition valid for all gen9 platforms.

v2: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
    to MOCS_ENTRIES. (Joonas)
v3 (Lucas):
  - Fix indentation
  - Rebase on rework done by additional patch
  - Remove define for or-ing flags as it made the table more complex by
    requiring zeroed values to be passed
  - Do not embed comma in the macro, so to treat that just as another
    item and please source code formatting tools

Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Tomasz Lis <tomasz.lis@intel.com> # v3
---
 drivers/gpu/drm/i915/intel_mocs.c | 57 ++++++++++++++-----------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 4ea80bb7dcc8..c7a2a8d81d90 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -93,46 +93,39 @@ struct drm_i915_mocs_table {
  *       may only be updated incrementally by adding entries at the
  *       end.
  */
+
+#define GEN9_MOCS_ENTRIES \
+	[I915_MOCS_UNCACHED] = { \
+		/* 0x00000009 */ \
+		.control_value = LE_1_UC | LE_TC_2_LLC_ELLC, \
+		/* 0x0010 */ \
+		.l3cc_value = L3_1_UC, \
+	}, \
+	[I915_MOCS_PTE] = { \
+		/* 0x00000038 */ \
+		.control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
+		/* 0x0030 */ \
+		.l3cc_value = L3_3_WB, \
+	}
+
 static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
-	[I915_MOCS_UNCACHED] = {
-	  /* 0x00000009 */
-	  .control_value = LE_1_UC | LE_TC_2_LLC_ELLC,
-	  /* 0x0010 */
-	  .l3cc_value =    L3_1_UC,
-	},
-	[I915_MOCS_PTE] = {
-	  /* 0x00000038 */
-	  .control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3),
-	  /* 0x0030 */
-	  .l3cc_value =    L3_3_WB,
-	},
+	GEN9_MOCS_ENTRIES,
 	[I915_MOCS_CACHED] = {
-	  /* 0x0000003b */
-	  .control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
-	  /* 0x0030 */
-	  .l3cc_value =   L3_3_WB,
+		/* 0x0000003b */
+		.control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+		/* 0x0030 */
+		.l3cc_value =   L3_3_WB,
 	},
 };
 
 /* NOTE: the LE_TGT_CACHE is not used on Broxton */
 static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
-	[I915_MOCS_UNCACHED] = {
-	  /* 0x00000009 */
-	  .control_value = LE_1_UC | LE_TC_2_LLC_ELLC,
-	  /* 0x0010 */
-	  .l3cc_value = L3_1_UC,
-	},
-	[I915_MOCS_PTE] = {
-	  /* 0x00000038 */
-	  .control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3),
-	  /* 0x0030 */
-	  .l3cc_value = L3_3_WB,
-	},
+	GEN9_MOCS_ENTRIES,
 	[I915_MOCS_CACHED] = {
-	  /* 0x00000039 */
-	  .control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
-	  /* 0x0030 */
-	  .l3cc_value = L3_3_WB,
+		/* 0x00000039 */
+		.control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+		/* 0x0030 */
+		.l3cc_value = L3_3_WB,
 	},
 };
 
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 4/7] drm/i915: use a macro to define MOCS entries

[Lucas De Marchi]

Let's use a macro to make tables smaller and at the same time allow us
to add fields that apply to all entries in future.

v2: rewrap lines to respect 80 chars limit and make it more readable
    (from Chris)

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 43 +++++++++++++------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index c7a2a8d81d90..59dd74765288 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -71,6 +71,12 @@ struct drm_i915_mocs_table {
 #define L3_2_RESERVED		_L3_CACHEABILITY(2)
 #define L3_3_WB			_L3_CACHEABILITY(3)
 
+#define MOCS_ENTRY(__idx, __control_value, __l3cc_value) \
+	[__idx] = { \
+		.control_value = __control_value, \
+		.l3cc_value = __l3cc_value, \
+	}
+
 /*
  * MOCS tables
  *
@@ -93,40 +99,27 @@ struct drm_i915_mocs_table {
  *       may only be updated incrementally by adding entries at the
  *       end.
  */
-
 #define GEN9_MOCS_ENTRIES \
-	[I915_MOCS_UNCACHED] = { \
-		/* 0x00000009 */ \
-		.control_value = LE_1_UC | LE_TC_2_LLC_ELLC, \
-		/* 0x0010 */ \
-		.l3cc_value = L3_1_UC, \
-	}, \
-	[I915_MOCS_PTE] = { \
-		/* 0x00000038 */ \
-		.control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
-		/* 0x0030 */ \
-		.l3cc_value = L3_3_WB, \
-	}
+	MOCS_ENTRY(I915_MOCS_UNCACHED, \
+		   LE_1_UC | LE_TC_2_LLC_ELLC, \
+		   L3_1_UC), \
+	MOCS_ENTRY(I915_MOCS_PTE, \
+		   LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
+		   L3_3_WB)
 
 static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
 	GEN9_MOCS_ENTRIES,
-	[I915_MOCS_CACHED] = {
-		/* 0x0000003b */
-		.control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
-		/* 0x0030 */
-		.l3cc_value =   L3_3_WB,
-	},
+	MOCS_ENTRY(I915_MOCS_CACHED,
+		   LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+		   L3_3_WB)
 };
 
 /* NOTE: the LE_TGT_CACHE is not used on Broxton */
 static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
 	GEN9_MOCS_ENTRIES,
-	[I915_MOCS_CACHED] = {
-		/* 0x00000039 */
-		.control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
-		/* 0x0030 */
-		.l3cc_value = L3_3_WB,
-	},
+	MOCS_ENTRY(I915_MOCS_CACHED,
+		   LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+		   L3_3_WB)
 };
 
 /**
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 5/7] drm/i915: keep track of used entries in MOCS table

[Lucas De Marchi]

Instead of considering we have defined entries for any index in the
table, let's keep track of the ones we explicitly defined. This will
allow Gen 11 to have it's new table defined in which we have holes of
undefined entries.

Repeated comments about the meaning of undefined entries were removed
since they are overly verbose and copy-pasted in several functions: now
the definition is in the top only.

v2: add helper function to get the index (from Chris)

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 111 +++++++++++++++++++++---------
 1 file changed, 78 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 59dd74765288..e5f0e9da258c 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -28,6 +28,7 @@
 struct drm_i915_mocs_entry {
 	u32 control_value;
 	u16 l3cc_value;
+	u16 used;
 };
 
 struct drm_i915_mocs_table {
@@ -75,6 +76,7 @@ struct drm_i915_mocs_table {
 	[__idx] = { \
 		.control_value = __control_value, \
 		.l3cc_value = __l3cc_value, \
+		.used = 1, \
 	}
 
 /*
@@ -187,6 +189,19 @@ static i915_reg_t mocs_register(enum intel_engine_id engine_id, int index)
 	}
 }
 
+/*
+ * Get control_value from MOCS entry taking into account when it's not used:
+ * I915_MOCS_PTE's value is returned in this case.
+ */
+static u32 get_entry_control(const struct drm_i915_mocs_table *table,
+			     unsigned int index)
+{
+	if (table->table[index].used)
+		return table->table[index].control_value;
+
+	return table->table[I915_MOCS_PTE].control_value;
+}
+
 /**
  * intel_mocs_init_engine() - emit the mocs control table
  * @engine:	The engine for whom to emit the registers.
@@ -199,24 +214,25 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	struct drm_i915_private *dev_priv = engine->i915;
 	struct drm_i915_mocs_table table;
 	unsigned int index;
+	u32 unused_value;
 
 	if (!get_mocs_settings(dev_priv, &table))
 		return;
 
 	GEM_BUG_ON(table.size > GEN9_NUM_MOCS_ENTRIES);
 
-	for (index = 0; index < table.size; index++)
-		I915_WRITE(mocs_register(engine->id, index),
-			   table.table[index].control_value);
+	/* Set unused values to PTE */
+	unused_value = table.table[I915_MOCS_PTE].control_value;
+
+	for (index = 0; index < table.size; index++) {
+		u32 value = get_entry_control(&table, index);
 
-	/*
-	 * Now set the unused entries to PTE. These entries are officially
-	 * undefined and no contract for the contents and settings is given
-	 * for these entries.
-	 */
+		I915_WRITE(mocs_register(engine->id, index), value);
+	}
+
+	/* All remaining entries are also unused */
 	for (; index < GEN9_NUM_MOCS_ENTRIES; index++)
-		I915_WRITE(mocs_register(engine->id, index),
-			   table.table[I915_MOCS_PTE].control_value);
+		I915_WRITE(mocs_register(engine->id, index), unused_value);
 }
 
 /**
@@ -234,11 +250,15 @@ static int emit_mocs_control_table(struct i915_request *rq,
 {
 	enum intel_engine_id engine = rq->engine->id;
 	unsigned int index;
+	u32 unused_value;
 	u32 *cs;
 
 	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
 		return -ENODEV;
 
+	/* Set unused values to PTE */
+	unused_value = table->table[I915_MOCS_PTE].control_value;
+
 	cs = intel_ring_begin(rq, 2 + 2 * GEN9_NUM_MOCS_ENTRIES);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
@@ -246,18 +266,16 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES);
 
 	for (index = 0; index < table->size; index++) {
+		u32 value = get_entry_control(table, index);
+
 		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
-		*cs++ = table->table[index].control_value;
+		*cs++ = value;
 	}
 
-	/*
-	 * Now set the unused entries to PTE. These entries are officially
-	 * undefined and no contract for the contents and settings is given
-	 * for these entries.
-	 */
+	/* All remaining entries are also unused */
 	for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
 		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
-		*cs++ = table->table[I915_MOCS_PTE].control_value;
+		*cs++ = unused_value;
 	}
 
 	*cs++ = MI_NOOP;
@@ -266,12 +284,24 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	return 0;
 }
 
+/*
+ * Get l3cc_value from MOCS entry taking into account when it's not used:
+ * I915_MOCS_PTE's value is returned in this case.
+ */
+static u16 get_entry_l3cc(const struct drm_i915_mocs_table *table,
+			  unsigned int index)
+{
+	if (table->table[index].used)
+		return table->table[index].l3cc_value;
+
+	return table->table[I915_MOCS_PTE].l3cc_value;
+}
+
 static inline u32 l3cc_combine(const struct drm_i915_mocs_table *table,
 			       u16 low,
 			       u16 high)
 {
-	return table->table[low].l3cc_value |
-	       table->table[high].l3cc_value << 16;
+	return low | high << 16;
 }
 
 /**
@@ -288,12 +318,16 @@ static inline u32 l3cc_combine(const struct drm_i915_mocs_table *table,
 static int emit_mocs_l3cc_table(struct i915_request *rq,
 				const struct drm_i915_mocs_table *table)
 {
+	u16 unused_value;
 	unsigned int i;
 	u32 *cs;
 
 	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
 		return -ENODEV;
 
+	/* Set unused values to PTE */
+	unused_value = table->table[I915_MOCS_PTE].l3cc_value;
+
 	cs = intel_ring_begin(rq, 2 + GEN9_NUM_MOCS_ENTRIES);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
@@ -301,25 +335,26 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2);
 
 	for (i = 0; i < table->size / 2; i++) {
+		u16 low = get_entry_l3cc(table, 2 * i);
+		u16 high = get_entry_l3cc(table, 2 * i + 1);
+
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, 2 * i, 2 * i + 1);
+		*cs++ = l3cc_combine(table, low, high);
 	}
 
+	/* Odd table size - 1 left over */
 	if (table->size & 0x01) {
-		/* Odd table size - 1 left over */
+		u16 low = get_entry_l3cc(table, 2 * i);
+
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, 2 * i, I915_MOCS_PTE);
+		*cs++ = l3cc_combine(table, low, unused_value);
 		i++;
 	}
 
-	/*
-	 * Now set the unused entries to PTE. These entries are officially
-	 * undefined and no contract for the contents and settings is given
-	 * for these entries.
-	 */
+	/* All remaining entries are also unused */
 	for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, I915_MOCS_PTE, I915_MOCS_PTE);
+		*cs++ = l3cc_combine(table, unused_value, unused_value);
 	}
 
 	*cs++ = MI_NOOP;
@@ -346,25 +381,35 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
 {
 	struct drm_i915_mocs_table table;
 	unsigned int i;
+	u16 unused_value;
 
 	if (!get_mocs_settings(dev_priv, &table))
 		return;
 
-	for (i = 0; i < table.size / 2; i++)
+	/* Set unused values to PTE */
+	unused_value = table.table[I915_MOCS_PTE].l3cc_value;
+
+	for (i = 0; i < table.size / 2; i++) {
+		u16 low = get_entry_l3cc(&table, 2 * i);
+		u16 high = get_entry_l3cc(&table, 2 * i + 1);
+
 		I915_WRITE(GEN9_LNCFCMOCS(i),
-			   l3cc_combine(&table, 2 * i, 2 * i + 1));
+			   l3cc_combine(&table, low, high));
+	}
 
 	/* Odd table size - 1 left over */
 	if (table.size & 0x01) {
+		u16 low = get_entry_l3cc(&table, 2 * i);
+
 		I915_WRITE(GEN9_LNCFCMOCS(i),
-			   l3cc_combine(&table, 2 * i, I915_MOCS_PTE));
+			   l3cc_combine(&table, low, unused_value));
 		i++;
 	}
 
-	/* Now set the rest of the table to PTE */
+	/* All remaining entries are also unused */
 	for (; i < (GEN9_NUM_MOCS_ENTRIES / 2); i++)
 		I915_WRITE(GEN9_LNCFCMOCS(i),
-			   l3cc_combine(&table, I915_MOCS_PTE, I915_MOCS_PTE));
+			   l3cc_combine(&table, unused_value, unused_value));
 }
 
 /**
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 6/7] drm/i915: cache number of MOCS entries

[Lucas De Marchi]

Instead of checking the gen number every time we need to know the max
number of entries, just save it into the table struct so we don't need
extra branches throughout the code. This will be useful for Ice Lake
that has 64 rather than 62 defined entries. Ice Lake changes will be
added in a follow up.

v2: make size and n_entries `unsigned int` and introduce changes as a
    pre-work for the Ice Lake changes (Tvrtko)

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index e5f0e9da258c..6e293f91a10c 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -32,7 +32,8 @@ struct drm_i915_mocs_entry {
 };
 
 struct drm_i915_mocs_table {
-	u32 size;
+	unsigned int size;
+	unsigned int n_entries;
 	const struct drm_i915_mocs_entry *table;
 };
 
@@ -144,10 +145,12 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
 	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
 	    IS_ICELAKE(dev_priv)) {
 		table->size  = ARRAY_SIZE(skylake_mocs_table);
+		table->n_entries = GEN9_NUM_MOCS_ENTRIES;
 		table->table = skylake_mocs_table;
 		result = true;
 	} else if (IS_GEN9_LP(dev_priv)) {
 		table->size  = ARRAY_SIZE(broxton_mocs_table);
+		table->n_entries = GEN9_NUM_MOCS_ENTRIES;
 		table->table = broxton_mocs_table;
 		result = true;
 	} else {
@@ -219,8 +222,6 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	if (!get_mocs_settings(dev_priv, &table))
 		return;
 
-	GEM_BUG_ON(table.size > GEN9_NUM_MOCS_ENTRIES);
-
 	/* Set unused values to PTE */
 	unused_value = table.table[I915_MOCS_PTE].control_value;
 
@@ -231,7 +232,7 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	}
 
 	/* All remaining entries are also unused */
-	for (; index < GEN9_NUM_MOCS_ENTRIES; index++)
+	for (; index < table.n_entries; index++)
 		I915_WRITE(mocs_register(engine->id, index), unused_value);
 }
 
@@ -253,17 +254,17 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	u32 unused_value;
 	u32 *cs;
 
-	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
+	if (GEM_WARN_ON(table->size > table->n_entries))
 		return -ENODEV;
 
 	/* Set unused values to PTE */
 	unused_value = table->table[I915_MOCS_PTE].control_value;
 
-	cs = intel_ring_begin(rq, 2 + 2 * GEN9_NUM_MOCS_ENTRIES);
+	cs = intel_ring_begin(rq, 2 + 2 * table->n_entries);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES);
+	*cs++ = MI_LOAD_REGISTER_IMM(table->n_entries);
 
 	for (index = 0; index < table->size; index++) {
 		u32 value = get_entry_control(table, index);
@@ -273,7 +274,7 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	}
 
 	/* All remaining entries are also unused */
-	for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
+	for (; index < table->n_entries; index++) {
 		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
 		*cs++ = unused_value;
 	}
@@ -322,17 +323,17 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 	unsigned int i;
 	u32 *cs;
 
-	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
+	if (GEM_WARN_ON(table->size > table->n_entries))
 		return -ENODEV;
 
 	/* Set unused values to PTE */
 	unused_value = table->table[I915_MOCS_PTE].l3cc_value;
 
-	cs = intel_ring_begin(rq, 2 + GEN9_NUM_MOCS_ENTRIES);
+	cs = intel_ring_begin(rq, 2 + table->n_entries);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2);
+	*cs++ = MI_LOAD_REGISTER_IMM(table->n_entries / 2);
 
 	for (i = 0; i < table->size / 2; i++) {
 		u16 low = get_entry_l3cc(table, 2 * i);
@@ -352,7 +353,7 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 	}
 
 	/* All remaining entries are also unused */
-	for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) {
+	for (; i < table->n_entries / 2; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
 		*cs++ = l3cc_combine(table, unused_value, unused_value);
 	}
@@ -407,7 +408,7 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
 	}
 
 	/* All remaining entries are also unused */
-	for (; i < (GEN9_NUM_MOCS_ENTRIES / 2); i++)
+	for (; i < table.n_entries / 2; i++)
 		I915_WRITE(GEN9_LNCFCMOCS(i),
 			   l3cc_combine(&table, unused_value, unused_value));
 }
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v9 7/7] drm/i915/icl: Define MOCS table for Icelake

[Lucas De Marchi]

From: Tomasz Lis <tomasz.lis@intel.com>

The table has been unified across OSes to minimize virtualization overhead.

The MOCS table is now published as part of bspec, and versioned. Entries
are supposed to never be modified, but new ones can be added. Adding
entries increases table version. The patch includes version 1 entries.

Meaning of each entry is now explained in bspec, and user mode clients
are expected to know what each entry means. The 3 entries used for previous
platforms are still compatible with their legacy definitions, but that is
not guaranteed to be true for future platforms.

v2: Fixed SCC values, improved commit comment (Daniele)
v3: Improved MOCS table comment (Daniele)
v4: Moved new entries below gen9 ones. Put common entries into
    definition to be used in multiple arrays. (Lucas)
v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
    to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
v6: Removed definitions of reserved entries. (Michal)
    Increased limit of entries sent to the hardware on gen11+.
v7: Simplify table as done for previou gens (Lucas)
v8: Rebase on cached number of entries per-platform and use new
    MOCS_ENTRY() macro (Lucas)
v9: Update comment (from Tomasz)

BSpec: 34007
BSpec: 560

Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Tomasz Lis <tomasz.lis@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 132 ++++++++++++++++++++++++++++--
 1 file changed, 123 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 6e293f91a10c..331e7a678fb7 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -46,6 +46,8 @@ struct drm_i915_mocs_table {
 #define LE_SCC(value)		((value) << 8)
 #define LE_PFM(value)		((value) << 11)
 #define LE_SCF(value)		((value) << 14)
+#define LE_COS(value)		((value) << 15)
+#define LE_SSE(value)		((value) << 17)
 
 /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
 #define L3_ESC(value)		((value) << 0)
@@ -54,6 +56,7 @@ struct drm_i915_mocs_table {
 
 /* Helper defines */
 #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. */
 
 /* (e)LLC caching options */
 #define LE_0_PAGETABLE		_LE_CACHEABILITY(0)
@@ -89,18 +92,23 @@ struct drm_i915_mocs_table {
  * LNCFCMOCS0 - LNCFCMOCS32 registers.
  *
  * These tables are intended to be kept reasonably consistent across
- * platforms. However some of the fields are not applicable to all of
- * them.
+ * HW platforms, and for ICL+, be identical across OSes. To achieve
+ * that, for Icelake and above, list of entries is published as part
+ * of bspec.
  *
  * Entries not part of the following tables are undefined as far as
  * userspace is concerned and shouldn't be relied upon.  For the time
  * being they will be initialized to PTE.
  *
- * NOTE: These tables MUST start with being uncached and the length
- *       MUST be less than 63 as the last two registers are reserved
- *       by the hardware.  These tables are part of the kernel ABI and
- *       may only be updated incrementally by adding entries at the
- *       end.
+ * The last two entries are reserved by the hardware. For ICL+ they
+ * should be initialized according to bspec and never used, for older
+ * platforms they should never be written to.
+ *
+ * NOTE: These tables are part of bspec and defined as part of hardware
+ *       interface for ICL+. For older platforms, they are part of kernel
+ *       ABI. It is expected that, for specific hardware platform, existing
+ *       entries will remain constant and the table will only be updated by
+ *       adding new entries, filling unused positions.
  */
 #define GEN9_MOCS_ENTRIES \
 	MOCS_ENTRY(I915_MOCS_UNCACHED, \
@@ -125,6 +133,108 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
 		   L3_3_WB)
 };
 
+#define GEN11_MOCS_ENTRIES \
+	/* Base - Uncached (Deprecated) */ \
+	MOCS_ENTRY(I915_MOCS_UNCACHED, \
+		   LE_1_UC | LE_TC_1_LLC, \
+		   L3_1_UC), \
+	/* Base - L3 + LeCC:PAT (Deprecated) */ \
+	MOCS_ENTRY(I915_MOCS_PTE, \
+		   LE_0_PAGETABLE | LE_TC_1_LLC, \
+		   L3_3_WB), \
+	/* Base - L3 + LLC */ \
+	MOCS_ENTRY(2, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_3_WB), \
+	/* Base - Uncached */ \
+	MOCS_ENTRY(3, \
+		   LE_1_UC | LE_TC_1_LLC, \
+		   L3_1_UC), \
+	/* Base - L3 */ \
+	MOCS_ENTRY(4, \
+		   LE_1_UC | LE_TC_1_LLC, \
+		   L3_3_WB), \
+	/* Base - LLC */ \
+	MOCS_ENTRY(5, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_1_UC), \
+	/* Age 0 - LLC */ \
+	MOCS_ENTRY(6, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1), \
+		   L3_1_UC), \
+	/* Age 0 - L3 + LLC */ \
+	MOCS_ENTRY(7, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1), \
+		   L3_3_WB), \
+	/* Age: Don't Chg. - LLC */ \
+	MOCS_ENTRY(8, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2), \
+		   L3_1_UC), \
+	/* Age: Don't Chg. - L3 + LLC */ \
+	MOCS_ENTRY(9, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2), \
+		   L3_3_WB), \
+	/* No AOM - LLC */ \
+	MOCS_ENTRY(10, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_AOM(1), \
+		   L3_1_UC), \
+	/* No AOM - L3 + LLC */ \
+	MOCS_ENTRY(11, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_AOM(1), \
+		   L3_3_WB), \
+	/* No AOM; Age 0 - LLC */ \
+	MOCS_ENTRY(12, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1) | LE_AOM(1), \
+		   L3_1_UC), \
+	/* No AOM; Age 0 - L3 + LLC */ \
+	MOCS_ENTRY(13, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1) | LE_AOM(1), \
+		   L3_3_WB), \
+	/* No AOM; Age:DC - LLC */ \
+	MOCS_ENTRY(14, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2) | LE_AOM(1), \
+		   L3_1_UC), \
+	/* No AOM; Age:DC - L3 + LLC */ \
+	MOCS_ENTRY(15, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2) | LE_AOM(1), \
+		   L3_3_WB), \
+	/* Self-Snoop - L3 + LLC */ \
+	MOCS_ENTRY(18, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SSE(3), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(12.5%) */ \
+	MOCS_ENTRY(19, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SCC(7), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(25%) */ \
+	MOCS_ENTRY(20, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SCC(3), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(50%) */ \
+	MOCS_ENTRY(21, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SCC(1), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(75%) */ \
+	MOCS_ENTRY(22, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(3), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(87.5%) */ \
+	MOCS_ENTRY(23, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
+		   L3_3_WB), \
+	/* HW Reserved - SW program but never use */ \
+	MOCS_ENTRY(62, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_1_UC), \
+	/* HW Reserved - SW program but never use */ \
+	MOCS_ENTRY(63, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_1_UC)
+
+static const struct drm_i915_mocs_entry icelake_mocs_table[] = {
+	GEN11_MOCS_ENTRIES
+};
+
 /**
  * get_mocs_settings()
  * @dev_priv:	i915 device.
@@ -142,8 +252,12 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
 {
 	bool result = false;
 
-	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
-	    IS_ICELAKE(dev_priv)) {
+	if (IS_ICELAKE(dev_priv)) {
+		table->size  = ARRAY_SIZE(icelake_mocs_table);
+		table->table = icelake_mocs_table;
+		table->n_entries = GEN11_NUM_MOCS_ENTRIES;
+		result = true;
+	} else if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
 		table->size  = ARRAY_SIZE(skylake_mocs_table);
 		table->n_entries = GEN9_NUM_MOCS_ENTRIES;
 		table->table = skylake_mocs_table;
-- 
2.20.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for Define MOCS table for Icelake (rev3)

[Patchwork]

== Series Details ==

Series: Define MOCS table for Icelake (rev3)
URL   : https://patchwork.freedesktop.org/series/54070/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
655b6926dbca drm/i915: initialize unused MOCS entries to PTE
2bef84e70dcf drm/i915: Simplify MOCS table definition
418e8e25ec07 drm/i915/skl: Rework MOCS tables to keep common part in a define
36b9b11d1b67 drm/i915: use a macro to define MOCS entries
7a770965049a drm/i915: keep track of used entries in MOCS table
126fe314fc9c drm/i915: cache number of MOCS entries
4ac13bc4fc3e drm/i915/icl: Define MOCS table for Icelake
-:95: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#95: FILE: drivers/gpu/drm/i915/intel_mocs.c:136:
+#define GEN11_MOCS_ENTRIES \
+	/* Base - Uncached (Deprecated) */ \
+	MOCS_ENTRY(I915_MOCS_UNCACHED, \
+		   LE_1_UC | LE_TC_1_LLC, \
+		   L3_1_UC), \
+	/* Base - L3 + LeCC:PAT (Deprecated) */ \
+	MOCS_ENTRY(I915_MOCS_PTE, \
+		   LE_0_PAGETABLE | LE_TC_1_LLC, \
+		   L3_3_WB), \
+	/* Base - L3 + LLC */ \
+	MOCS_ENTRY(2, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_3_WB), \
+	/* Base - Uncached */ \
+	MOCS_ENTRY(3, \
+		   LE_1_UC | LE_TC_1_LLC, \
+		   L3_1_UC), \
+	/* Base - L3 */ \
+	MOCS_ENTRY(4, \
+		   LE_1_UC | LE_TC_1_LLC, \
+		   L3_3_WB), \
+	/* Base - LLC */ \
+	MOCS_ENTRY(5, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_1_UC), \
+	/* Age 0 - LLC */ \
+	MOCS_ENTRY(6, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1), \
+		   L3_1_UC), \
+	/* Age 0 - L3 + LLC */ \
+	MOCS_ENTRY(7, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1), \
+		   L3_3_WB), \
+	/* Age: Don't Chg. - LLC */ \
+	MOCS_ENTRY(8, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2), \
+		   L3_1_UC), \
+	/* Age: Don't Chg. - L3 + LLC */ \
+	MOCS_ENTRY(9, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2), \
+		   L3_3_WB), \
+	/* No AOM - LLC */ \
+	MOCS_ENTRY(10, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_AOM(1), \
+		   L3_1_UC), \
+	/* No AOM - L3 + LLC */ \
+	MOCS_ENTRY(11, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_AOM(1), \
+		   L3_3_WB), \
+	/* No AOM; Age 0 - LLC */ \
+	MOCS_ENTRY(12, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1) | LE_AOM(1), \
+		   L3_1_UC), \
+	/* No AOM; Age 0 - L3 + LLC */ \
+	MOCS_ENTRY(13, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(1) | LE_AOM(1), \
+		   L3_3_WB), \
+	/* No AOM; Age:DC - LLC */ \
+	MOCS_ENTRY(14, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2) | LE_AOM(1), \
+		   L3_1_UC), \
+	/* No AOM; Age:DC - L3 + LLC */ \
+	MOCS_ENTRY(15, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(2) | LE_AOM(1), \
+		   L3_3_WB), \
+	/* Self-Snoop - L3 + LLC */ \
+	MOCS_ENTRY(18, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SSE(3), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(12.5%) */ \
+	MOCS_ENTRY(19, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SCC(7), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(25%) */ \
+	MOCS_ENTRY(20, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SCC(3), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(50%) */ \
+	MOCS_ENTRY(21, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SCC(1), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(75%) */ \
+	MOCS_ENTRY(22, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(3), \
+		   L3_3_WB), \
+	/* Skip Caching - L3 + LLC(87.5%) */ \
+	MOCS_ENTRY(23, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
+		   L3_3_WB), \
+	/* HW Reserved - SW program but never use */ \
+	MOCS_ENTRY(62, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_1_UC), \
+	/* HW Reserved - SW program but never use */ \
+	MOCS_ENTRY(63, \
+		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+		   L3_1_UC)

total: 1 errors, 0 warnings, 0 checks, 167 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Define MOCS table for Icelake (rev3)

[Patchwork]

== Series Details ==

Series: Define MOCS table for Icelake (rev3)
URL   : https://patchwork.freedesktop.org/series/54070/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5471 -> Patchwork_12021
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54070/revisions/3/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12021 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362] +1

  
#### Possible fixes ####

  * igt@kms_busy@basic-flip-c:
    - fi-kbl-7500u:       {SKIP} [fdo#109271] / [fdo#109278] -> PASS +2

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-kbl-7500u:       DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105602] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (46 -> 40)
------------------------------

  Missing    (6): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5471 -> Patchwork_12021

  CI_DRM_5471: 198addb18e12d2469bc41d57f9ed63e1072a7f82 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4786: 85cf76182087c09604bcae2bbee9e58b33bcb4f2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12021: 4ac13bc4fc3e741d74d284d05de8fde046c9debf @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4ac13bc4fc3e drm/i915/icl: Define MOCS table for Icelake
126fe314fc9c drm/i915: cache number of MOCS entries
7a770965049a drm/i915: keep track of used entries in MOCS table
36b9b11d1b67 drm/i915: use a macro to define MOCS entries
418e8e25ec07 drm/i915/skl: Rework MOCS tables to keep common part in a define
2bef84e70dcf drm/i915: Simplify MOCS table definition
655b6926dbca drm/i915: initialize unused MOCS entries to PTE

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12021/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Define MOCS table for Icelake (rev3)

[Patchwork]

== Series Details ==

Series: Define MOCS table for Icelake (rev3)
URL   : https://patchwork.freedesktop.org/series/54070/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5471_full -> Patchwork_12021_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12021_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_get_client_auth@master-drop:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-apl:          PASS -> FAIL [fdo#106510] / [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          FAIL [fdo#106510] / [fdo#108145] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-kbl:          DMESG-WARN [fdo#109244] -> INCOMPLETE [fdo#103665] / [fdo#106886]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5471 -> Patchwork_12021

  CI_DRM_5471: 198addb18e12d2469bc41d57f9ed63e1072a7f82 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4786: 85cf76182087c09604bcae2bbee9e58b33bcb4f2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12021: 4ac13bc4fc3e741d74d284d05de8fde046c9debf @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12021/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v9 4/7] drm/i915: use a macro to define MOCS entries

[Chris Wilson]

Quoting Lucas De Marchi (2019-01-24 00:06:01)
> Let's use a macro to make tables smaller and at the same time allow us
> to add fields that apply to all entries in future.
> 
> v2: rewrap lines to respect 80 chars limit and make it more readable
>     (from Chris)
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v9 5/7] drm/i915: keep track of used entries in MOCS table

[Chris Wilson]

Quoting Lucas De Marchi (2019-01-24 00:06:02)
> Instead of considering we have defined entries for any index in the
> table, let's keep track of the ones we explicitly defined. This will
> allow Gen 11 to have it's new table defined in which we have holes of
> undefined entries.
> 
> Repeated comments about the meaning of undefined entries were removed
> since they are overly verbose and copy-pasted in several functions: now
> the definition is in the top only.
> 
> v2: add helper function to get the index (from Chris)
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v9 7/7] drm/i915/icl: Define MOCS table for Icelake

[Joonas Lahtinen]

Quoting Lucas De Marchi (2019-01-24 02:06:04)
> From: Tomasz Lis <tomasz.lis@intel.com>
> 
> The table has been unified across OSes to minimize virtualization overhead.
> 
> The MOCS table is now published as part of bspec, and versioned. Entries
> are supposed to never be modified, but new ones can be added. Adding
> entries increases table version. The patch includes version 1 entries.
> 
> Meaning of each entry is now explained in bspec, and user mode clients
> are expected to know what each entry means. The 3 entries used for previous
> platforms are still compatible with their legacy definitions, but that is
> not guaranteed to be true for future platforms.
> 
> v2: Fixed SCC values, improved commit comment (Daniele)
> v3: Improved MOCS table comment (Daniele)
> v4: Moved new entries below gen9 ones. Put common entries into
>     definition to be used in multiple arrays. (Lucas)
> v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
>     to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
> v6: Removed definitions of reserved entries. (Michal)
>     Increased limit of entries sent to the hardware on gen11+.
> v7: Simplify table as done for previou gens (Lucas)
> v8: Rebase on cached number of entries per-platform and use new
>     MOCS_ENTRY() macro (Lucas)
> v9: Update comment (from Tomasz)
> 
> BSpec: 34007
> BSpec: 560
> 
> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Acked-by: Tomasz Lis <tomasz.lis@intel.com>

I don't think we should have A-b/R-b from patch authors.

If you add you S-o-b to the code, you can remove the R-b, and if you
wrote portion of the code, you don't really add Reviewed-by or Acked-by.

Similarly, if the code is modified after some R-bs are given, you should
indicate those reviews to be stale.

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v9 7/7] drm/i915/icl: Define MOCS table for Icelake

[Lucas De Marchi]

On Thu, Jan 24, 2019 at 12:41:27PM +0200, Joonas Lahtinen wrote:
>Quoting Lucas De Marchi (2019-01-24 02:06:04)
>> From: Tomasz Lis <tomasz.lis@intel.com>
>>
>> The table has been unified across OSes to minimize virtualization overhead.
>>
>> The MOCS table is now published as part of bspec, and versioned. Entries
>> are supposed to never be modified, but new ones can be added. Adding
>> entries increases table version. The patch includes version 1 entries.
>>
>> Meaning of each entry is now explained in bspec, and user mode clients
>> are expected to know what each entry means. The 3 entries used for previous
>> platforms are still compatible with their legacy definitions, but that is
>> not guaranteed to be true for future platforms.
>>
>> v2: Fixed SCC values, improved commit comment (Daniele)
>> v3: Improved MOCS table comment (Daniele)
>> v4: Moved new entries below gen9 ones. Put common entries into
>>     definition to be used in multiple arrays. (Lucas)
>> v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
>>     to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
>> v6: Removed definitions of reserved entries. (Michal)
>>     Increased limit of entries sent to the hardware on gen11+.
>> v7: Simplify table as done for previou gens (Lucas)
>> v8: Rebase on cached number of entries per-platform and use new
>>     MOCS_ENTRY() macro (Lucas)
>> v9: Update comment (from Tomasz)
>>
>> BSpec: 34007
>> BSpec: 560
>>
>> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Acked-by: Tomasz Lis <tomasz.lis@intel.com>
>
>I don't think we should have A-b/R-b from patch authors.

In this case I thought it was good because I modified right and left the
patch from someone else. Getting and ack from the original author is
good.

>
>If you add you S-o-b to the code, you can remove the R-b, and if you
>wrote portion of the code, you don't really add Reviewed-by or Acked-by.

ok

>
>Similarly, if the code is modified after some R-bs are given, you should
>indicate those reviews to be stale.

Daniele had reviewed a very recent one - later changes were mostly
cosmetics. Anyway, I asked Daniele on irc after seing your response and
he said his r-b still stands, so I'm keeping it.

thanks
Lucas De Marchi

>
>Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx