Re: [PATCH 19/22] drm/i915/tgl: Tigerlake only has global MOCS registers

Lucas De Marchi <lucas.demarchi@xxxxxxxxx> · Fri, 19 Jul 2019 12:42:43 -0700

On Thu, Jul 18, 2019 at 10:40:43AM -0700, Daniele Ceraolo Spurio wrote:


On 7/12/19 6:09 PM, Lucas De Marchi wrote:
From: Michel Thierry <michel.thierry@xxxxxxxxx>

Until Icelake, each engine had its own set of 64 MOCS registers. In
order to simplify, Tigerlake moves to only 64 Global MOCS registers,
which are no longer part of the engine context. Since these registers
are now global, they also only need to be initialized once.

These new global MOCS registers are located in the same offset of the
render MOCS register from previous platforms.

No, they're not :). They're located in a new range, which overlaps 
with the location where the fault register was previously located and 
that's why the offset of that one has been changed as well. The code 
does the right thing, so only this sentence needs fixing.

the FAULT_TLB_DATA* registers have been moved as well as part of the 
re-org, so maybe we can have a patch that just moves all the offsets 
and then add the global mocs on top?

yep, makes sense. I will change that in the next version.



From Gen12 onwards, MOCS must specify the target cache (3:2) and LRU
management (5:4) fields and cannot be programmed to 'use the value from
Private PAT', because these fields are no longer part of the PPAT. Also
cacheability control (1:0) field has changed, 00 no longer means 'use
controls from page table', but uncacheable (UC).

Should we put this as a comment in the code somewhere? Although I'm 
not sure we have any use for this info since we copy the table 
straight from the specs and we should probably trust it is correct.

I don't see a value in the comment exactly because of this reason. I
think the one in the commit message is enough.

thanks
Lucas De Marchi


Daniele


Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Tomasz Lis <tomasz.lis@xxxxxxxxx>
Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
 drivers/gpu/drm/i915/gt/intel_gt.c       | 12 ++++--
 drivers/gpu/drm/i915/gt/intel_mocs.c     | 47 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_mocs.h     |  1 +
 drivers/gpu/drm/i915/i915_drv.h          |  2 +
 drivers/gpu/drm/i915/i915_gem.c          |  1 +
 drivers/gpu/drm/i915/i915_gpu_error.c    | 11 ++++--
 drivers/gpu/drm/i915/i915_pci.c          |  3 +-
 drivers/gpu/drm/i915/i915_reg.h          |  3 ++
 drivers/gpu/drm/i915/intel_device_info.h |  1 +
 9 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index f7e69db4019d..958edfda2ba2 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -80,8 +80,11 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 	}
 	if (INTEL_GEN(i915) >= 8) {
-		rmw_clear(uncore, GEN8_RING_FAULT_REG, RING_FAULT_VALID);
-		intel_uncore_posting_read(uncore, GEN8_RING_FAULT_REG);
+		i915_reg_t fault_reg = (INTEL_GEN(i915) >= 12) ?
+					GEN12_RING_FAULT_REG :
+					GEN8_RING_FAULT_REG;
+		rmw_clear(uncore, fault_reg, RING_FAULT_VALID);
+		intel_uncore_posting_read(uncore, fault_reg);
 	} else if (INTEL_GEN(i915) >= 6) {
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
@@ -117,7 +120,10 @@ static void gen6_check_faults(struct intel_gt *gt)
 static void gen8_check_faults(struct intel_gt *gt)
 {
 	struct intel_uncore *uncore = gt->uncore;
-	u32 fault = intel_uncore_read(uncore, GEN8_RING_FAULT_REG);
+	i915_reg_t fault_reg =
+		(INTEL_GEN(gt->i915) >= 12) ?
+		GEN12_RING_FAULT_REG : GEN8_RING_FAULT_REG;
+	u32 fault = intel_uncore_read(uncore, fault_reg);
 	if (fault & RING_FAULT_VALID) {
 		u32 fault_data0, fault_data1;
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 259e7bec0a63..365d8ff11f23 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -365,6 +365,10 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	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);
@@ -389,6 +393,46 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 				      unused_value);
 }
+/**
+ * intel_mocs_init_global() - program the global mocs registers
+ * gt:      pointer to struct intel_gt
+ *
+ * This function initializes the MOCS global registers.
+ */
+void intel_mocs_init_global(struct intel_gt *gt)
+{
+	struct intel_uncore *uncore = gt->uncore;
+	struct drm_i915_mocs_table table;
+	unsigned int index;
+
+	if (!HAS_GLOBAL_MOCS_REGISTERS(gt->i915))
+		return;
+
+	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 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.
+	 */
+	for (; index < table.n_entries; index++)
+		intel_uncore_write(uncore,
+				   GEN12_GLOBAL_MOCS(index),
+				   table.table[0].control_value);
+}
+
 /**
  * emit_mocs_control_table() - emit the mocs control table
  * @rq:	Request to set up the MOCS table for.
@@ -592,6 +636,9 @@ int intel_rcs_context_init_mocs(struct i915_request *rq)
 	struct drm_i915_mocs_table t;
 	int ret;
+	if (HAS_GLOBAL_MOCS_REGISTERS(rq->i915))
+		return 0;
+
 	if (get_mocs_settings(rq->engine->gt, &t)) {
 		/* Program the RCS control registers */
 		ret = emit_mocs_control_table(rq, &t);
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.h b/drivers/gpu/drm/i915/gt/intel_mocs.h
index 8b9813e6f9ac..aa3a2df07c82 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.h
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.h
@@ -56,6 +56,7 @@ struct intel_gt;
 int intel_rcs_context_init_mocs(struct i915_request *rq);
 void intel_mocs_init_l3cc_table(struct intel_gt *gt);
+void intel_mocs_init_global(struct intel_gt *gt);
 void intel_mocs_init_engine(struct intel_engine_cs *engine);
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 069337f11872..57c05650d3b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2308,6 +2308,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define HAS_POOLED_EU(dev_priv)	(INTEL_INFO(dev_priv)->has_pooled_eu)
+#define HAS_GLOBAL_MOCS_REGISTERS(dev_priv)	(INTEL_INFO(dev_priv)->has_global_mocs)
+
 #define INTEL_PCH_DEVICE_ID_MASK		0xff80
 #define INTEL_PCH_IBX_DEVICE_ID_TYPE		0x3b00
 #define INTEL_PCH_CPT_DEVICE_ID_TYPE		0x1c00
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e24955b5ebc2..d23e156f6659 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1254,6 +1254,7 @@ int i915_gem_init_hw(struct drm_i915_private *i915)
 		goto out;
 	}
+	intel_mocs_init_global(gt);
 	intel_mocs_init_l3cc_table(gt);
 	intel_engines_set_scheduler_caps(i915);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 5489cd879315..ee40c14caa64 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1153,7 +1153,10 @@ static void error_record_engine_registers(struct i915_gpu_state *error,
 	if (INTEL_GEN(dev_priv) >= 6) {
 		ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
-		if (INTEL_GEN(dev_priv) >= 8)
+
+		if (INTEL_GEN(dev_priv) >= 12)
+			ee->fault_reg = I915_READ(GEN12_RING_FAULT_REG);
+		else if (INTEL_GEN(dev_priv) >= 8)
 			ee->fault_reg = I915_READ(GEN8_RING_FAULT_REG);
 		else
 			ee->fault_reg = GEN6_RING_FAULT_REG_READ(engine);
@@ -1613,8 +1616,10 @@ static void capture_reg_state(struct i915_gpu_state *error)
 	if (INTEL_GEN(i915) >= 6) {
 		error->derrmr = intel_uncore_read(uncore, DERRMR);
-		error->error = intel_uncore_read(uncore, ERROR_GEN6);
-		error->done_reg = intel_uncore_read(uncore, DONE_REG);
+		if (INTEL_GEN(i915) < 12) {
+			error->error = intel_uncore_read(uncore, ERROR_GEN6);
+			error->done_reg = intel_uncore_read(uncore, DONE_REG);
+		}
 	}
 	if (INTEL_GEN(i915) >= 5)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 40076ba431d4..f0e19ef2e71a 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -783,7 +783,8 @@ static const struct intel_device_info intel_elkhartlake_info = {
 		[TRANSCODER_D] = TRANSCODER_D_OFFSET, \
 		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
 		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
-	}
+	}, \
+	.has_global_mocs = 1
 static const struct intel_device_info intel_tigerlake_12_info = {
 	GEN12_FEATURES,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 712616fcd6b3..6cfcdf6bb1bb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2490,6 +2490,7 @@ enum i915_power_well_id {
 #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
 #define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
 #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
+#define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
 #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
 #define   RING_FAULT_GTTSEL_MASK (1 << 11)
 #define   RING_FAULT_SRCID(x)	(((x) >> 3) & 0xff)
@@ -11362,6 +11363,8 @@ enum skl_power_gate {
 #define   PMFLUSH_GAPL3UNBLOCK		(1 << 21)
 #define   PMFLUSHDONE_LNEBLK		(1 << 22)
+#define GEN12_GLOBAL_MOCS(i)	_MMIO(0x4000 + (i) * 4) /* Global MOCS regs */
+
 /* gamt regs */
 #define GEN8_L3_LRA_1_GPGPU _MMIO(0x4dd4)
 #define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW  0x67F1427F /* max/min for LRA1/2 */
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 45a9badc9b8e..aea7d143ec47 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -112,6 +112,7 @@ enum intel_ppgtt_type {
 	func(gpu_reset_clobbers_display); \
 	func(has_reset_engine); \
 	func(has_fpga_dbg); \
+	func(has_global_mocs); \
 	func(has_guc); \
 	func(has_l3_dpf); \
 	func(has_llc); \

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







