[PATCH] drm/i915: Do initial mocs configuration directly

* [PATCH] drm/i915: Do initial mocs configuration directly
@ 2019-10-16  9:07 Chris Wilson
  2019-10-16 10:24 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Chris Wilson @ 2019-10-16  9:07 UTC (permalink / raw)
  To: intel-gfx

Now that we record the default "goldenstate" context, we do not need to
emit the mocs registers at the start of each context and can simply do
mmio before the first context and capture the registers as part of its
default image. As a consequence, this means that we repeat the mmio
after each engine reset, fixing up any platform and registers that were
zapped by the reset (for those platforms with global not context-saved
settings).

References: https://bugs.freedesktop.org/show_bug.cgi?id=111723
References: https://bugs.freedesktop.org/show_bug.cgi?id=111645
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_mocs.c | 281 +++++++--------------------
 drivers/gpu/drm/i915/gt/intel_mocs.h |   3 -
 drivers/gpu/drm/i915/i915_gem.c      |   9 -
 3 files changed, 73 insertions(+), 220 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 728704bbbe18..ee150b034855 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -287,10 +287,9 @@ static const struct drm_i915_mocs_entry icelake_mocs_table[] = {
 	GEN11_MOCS_ENTRIES
 };
 
-static bool get_mocs_settings(struct intel_gt *gt,
+static bool get_mocs_settings(const struct drm_i915_private *i915,
 			      struct drm_i915_mocs_table *table)
 {
-	struct drm_i915_private *i915 = gt->i915;
 	bool result = false;
 
 	if (INTEL_GEN(i915) >= 12) {
@@ -331,9 +330,9 @@ static bool get_mocs_settings(struct intel_gt *gt,
 	return result;
 }
 
-static i915_reg_t mocs_register(enum intel_engine_id engine_id, int index)
+static i915_reg_t mocs_register(const struct intel_engine_cs *engine, int index)
 {
-	switch (engine_id) {
+	switch (engine->id) {
 	case RCS0:
 		return GEN9_GFX_MOCS(index);
 	case VCS0:
@@ -347,7 +346,7 @@ static i915_reg_t mocs_register(enum intel_engine_id engine_id, int index)
 	case VCS2:
 		return GEN11_MFX2_MOCS(index);
 	default:
-		MISSING_CASE(engine_id);
+		MISSING_CASE(engine->id);
 		return INVALID_MMIO_REG;
 	}
 }
@@ -365,118 +364,25 @@ static u32 get_entry_control(const struct drm_i915_mocs_table *table,
 	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.
- *
- * This function simply emits a MI_LOAD_REGISTER_IMM command for the
- * given table starting at the given address.
- */
-void intel_mocs_init_engine(struct intel_engine_cs *engine)
+static void init_mocs_table(struct intel_engine_cs *engine,
+			    const struct drm_i915_mocs_table *table)
 {
-	struct intel_gt *gt = engine->gt;
-	struct intel_uncore *uncore = gt->uncore;
-	struct drm_i915_mocs_table table;
-	unsigned int index;
-	u32 unused_value;
-
-	/* Platforms with global MOCS do not need per-engine initialization. */
-	if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
-		return;
-
-	/* Called under a blanket forcewake */
-	assert_forcewakes_active(uncore, FORCEWAKE_ALL);
-
-	if (!get_mocs_settings(gt, &table))
-		return;
-
-	/* 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);
+	struct intel_uncore *uncore = engine->uncore;
+	u32 unused_value = table->table[I915_MOCS_PTE].control_value;
+	unsigned int i;
 
+	for (i = 0; i < table->size; i++)
 		intel_uncore_write_fw(uncore,
-				      mocs_register(engine->id, index),
-				      value);
-	}
+				      mocs_register(engine, i),
+				      get_entry_control(table, i));
 
-	/* All remaining entries are also unused */
-	for (; index < table.n_entries; index++)
+	/* All remaining entries are unused */
+	for (; i < table->n_entries; i++)
 		intel_uncore_write_fw(uncore,
-				      mocs_register(engine->id, index),
+				      mocs_register(engine, i),
 				      unused_value);
 }
 
-static void intel_mocs_init_global(struct intel_gt *gt)
-{
-	struct intel_uncore *uncore = gt->uncore;
-	struct drm_i915_mocs_table table;
-	unsigned int index;
-
-	GEM_BUG_ON(!HAS_GLOBAL_MOCS_REGISTERS(gt->i915));
-
-	if (!get_mocs_settings(gt, &table))
-		return;
-
-	if (GEM_DEBUG_WARN_ON(table.size > table.n_entries))
-		return;
-
-	for (index = 0; index < table.size; index++)
-		intel_uncore_write(uncore,
-				   GEN12_GLOBAL_MOCS(index),
-				   table.table[index].control_value);
-
-	/*
-	 * Ok, now set the unused entries to the invalid entry (index 0). These
-	 * entries are officially undefined and no contract for the contents and
-	 * settings is given for these entries.
-	 */
-	for (; index < table.n_entries; index++)
-		intel_uncore_write(uncore,
-				   GEN12_GLOBAL_MOCS(index),
-				   table.table[0].control_value);
-}
-
-static int emit_mocs_control_table(struct i915_request *rq,
-				   const struct drm_i915_mocs_table *table)
-{
-	enum intel_engine_id engine = rq->engine->id;
-	unsigned int index;
-	u32 unused_value;
-	u32 *cs;
-
-	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 * table->n_entries);
-	if (IS_ERR(cs))
-		return PTR_ERR(cs);
-
-	*cs++ = MI_LOAD_REGISTER_IMM(table->n_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++ = value;
-	}
-
-	/* All remaining entries are also unused */
-	for (; index < table->n_entries; index++) {
-		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
-		*cs++ = unused_value;
-	}
-
-	*cs++ = MI_NOOP;
-	intel_ring_advance(rq, cs);
-
-	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.
@@ -494,141 +400,100 @@ static inline u32 l3cc_combine(const struct drm_i915_mocs_table *table,
 			       u16 low,
 			       u16 high)
 {
-	return low | high << 16;
+	return low | (u32)high << 16;
 }
 
-static int emit_mocs_l3cc_table(struct i915_request *rq,
-				const struct drm_i915_mocs_table *table)
+static void init_l3cc_table(struct intel_engine_cs *engine,
+			    const struct drm_i915_mocs_table *table)
 {
-	u16 unused_value;
+	struct intel_uncore *uncore = engine->uncore;
+	u16 unused_value = table->table[I915_MOCS_PTE].l3cc_value;
 	unsigned int i;
-	u32 *cs;
-
-	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 + table->n_entries);
-	if (IS_ERR(cs))
-		return PTR_ERR(cs);
-
-	*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);
 		u16 high = get_entry_l3cc(table, 2 * i + 1);
 
-		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, low, high);
+		intel_uncore_write(uncore,
+				   GEN9_LNCFCMOCS(i),
+				   l3cc_combine(table, low, high));
 	}
 
 	/* Odd table size - 1 left over */
-	if (table->size & 0x01) {
+	if (table->size & 1) {
 		u16 low = get_entry_l3cc(table, 2 * i);
 
-		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, low, unused_value);
+		intel_uncore_write(uncore,
+				   GEN9_LNCFCMOCS(i),
+				   l3cc_combine(table, low, unused_value));
 		i++;
 	}
 
 	/* All remaining entries are also unused */
-	for (; i < table->n_entries / 2; i++) {
-		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
-		*cs++ = l3cc_combine(table, unused_value, unused_value);
-	}
-
-	*cs++ = MI_NOOP;
-	intel_ring_advance(rq, cs);
-
-	return 0;
+	for (; i < table->n_entries / 2; i++)
+		intel_uncore_write(uncore,
+				   GEN9_LNCFCMOCS(i),
+				   l3cc_combine(table, unused_value,
+						unused_value));
 }
 
-static void intel_mocs_init_l3cc_table(struct intel_gt *gt)
+/**
+ * intel_mocs_init_engine() - emit the mocs control table
+ * @engine:	The engine for whom to emit the registers.
+ *
+ * This function simply emits a MI_LOAD_REGISTER_IMM command for the
+ * given table starting at the given address.
+ */
+void intel_mocs_init_engine(struct intel_engine_cs *engine)
 {
-	struct intel_uncore *uncore = gt->uncore;
 	struct drm_i915_mocs_table table;
-	unsigned int i;
-	u16 unused_value;
 
-	if (!get_mocs_settings(gt, &table))
+	/* Called under a blanket forcewake */
+	assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
+
+	if (!get_mocs_settings(engine->i915, &table))
 		return;
 
-	/* Set unused values to PTE */
-	unused_value = table.table[I915_MOCS_PTE].l3cc_value;
+	/* Platforms with global MOCS do not need per-engine initialization. */
+	if (!HAS_GLOBAL_MOCS_REGISTERS(engine->i915))
+		init_mocs_table(engine, &table);
 
-	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);
+	if (engine->class == RENDER_CLASS)
+		init_l3cc_table(engine, &table);
+}
 
-		intel_uncore_write(uncore,
-				   GEN9_LNCFCMOCS(i),
-				   l3cc_combine(&table, low, high));
-	}
+static void intel_mocs_init_global(struct intel_gt *gt)
+{
+	struct intel_uncore *uncore = gt->uncore;
+	struct drm_i915_mocs_table table;
+	unsigned int index;
 
-	/* Odd table size - 1 left over */
-	if (table.size & 0x01) {
-		u16 low = get_entry_l3cc(&table, 2 * i);
+	GEM_BUG_ON(!HAS_GLOBAL_MOCS_REGISTERS(gt->i915));
 
-		intel_uncore_write(uncore,
-				   GEN9_LNCFCMOCS(i),
-				   l3cc_combine(&table, low, unused_value));
-		i++;
-	}
+	if (!get_mocs_settings(gt->i915, &table))
+		return;
 
-	/* All remaining entries are also unused */
-	for (; i < table.n_entries / 2; i++)
-		intel_uncore_write(uncore,
-				   GEN9_LNCFCMOCS(i),
-				   l3cc_combine(&table, unused_value,
-						unused_value));
-}
+	if (GEM_DEBUG_WARN_ON(table.size > table.n_entries))
+		return;
 
-/**
- * intel_mocs_emit() - program the MOCS register.
- * @rq:	Request to use to set up the MOCS tables.
- *
- * This function will emit a batch buffer with the values required for
- * programming the MOCS register values for all the currently supported
- * rings.
- *
- * These registers are partially stored in the RCS context, so they are
- * emitted at the same time so that when a context is created these registers
- * are set up. These registers have to be emitted into the start of the
- * context as setting the ELSP will re-init some of these registers back
- * to the hw values.
- *
- * Return: 0 on success, otherwise the error status.
- */
-int intel_mocs_emit(struct i915_request *rq)
-{
-	struct drm_i915_mocs_table t;
-	int ret;
-
-	if (HAS_GLOBAL_MOCS_REGISTERS(rq->i915) ||
-	    rq->engine->class != RENDER_CLASS)
-		return 0;
-
-	if (get_mocs_settings(rq->engine->gt, &t)) {
-		/* Program the RCS control registers */
-		ret = emit_mocs_control_table(rq, &t);
-		if (ret)
-			return ret;
-
-		/* Now program the l3cc registers */
-		ret = emit_mocs_l3cc_table(rq, &t);
-		if (ret)
-			return ret;
-	}
+	for (index = 0; index < table.size; index++)
+		intel_uncore_write(uncore,
+				   GEN12_GLOBAL_MOCS(index),
+				   table.table[index].control_value);
 
-	return 0;
+	/*
+	 * Ok, now set the unused entries to the invalid entry (index 0). These
+	 * entries are officially undefined and no contract for the contents and
+	 * settings is given for these entries.
+	 */
+	for (; index < table.n_entries; index++)
+		intel_uncore_write(uncore,
+				   GEN12_GLOBAL_MOCS(index),
+				   table.table[0].control_value);
 }
 
 void intel_mocs_init(struct intel_gt *gt)
 {
-	intel_mocs_init_l3cc_table(gt);
-
 	if (HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
 		intel_mocs_init_global(gt);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.h b/drivers/gpu/drm/i915/gt/intel_mocs.h
index 2ae816b7ca19..83371f3e6ba1 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.h
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.h
@@ -49,13 +49,10 @@
  * context handling keep the MOCS in step.
  */
 
-struct i915_request;
 struct intel_engine_cs;
 struct intel_gt;
 
 void intel_mocs_init(struct intel_gt *gt);
 void intel_mocs_init_engine(struct intel_engine_cs *engine);
 
-int intel_mocs_emit(struct i915_request *rq);
-
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0ddbd3a5fb8d..3694c7ff6d23 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1111,15 +1111,6 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
 		if (err)
 			goto err_rq;
 
-		/*
-		 * Failing to program the MOCS is non-fatal.The system will not
-		 * run at peak performance. So warn the user and carry on.
-		 */
-		err = intel_mocs_emit(rq);
-		if (err)
-			dev_notice(i915->drm.dev,
-				   "Failed to program MOCS registers; expect performance issues.\n");
-
 		err = intel_renderstate_emit(rq);
 		if (err)
 			goto err_rq;
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread