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

Lucas De Marchi lucas.demarchi at intel.com
Thu Jan 24 00:05:58 UTC 2019


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 at intel.com>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
Reviewed-by: Tomasz Lis <tomasz.lis at 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



More information about the Intel-gfx mailing list