[PATCH 0/2] i915: CAGF and RC6 changes for MTL

[PATCH 0/2] i915: CAGF and RC6 changes for MTL

[Badal Nilawar]

This series includes the code changes to get CAGF, RC State and 
C6 Residency of MTL.

v2: Included "Use GEN12 RPSTAT register" patch 

v3: 
  - Rebased
  - Dropped "Use GEN12 RPSTAT register" patch from this series
    going to send separate series for it

Badal Nilawar (2):
  drm/i915/mtl: Modify CAGF functions for MTL
  drm/i915/mtl: Add C6 residency support for MTL SAMedia

 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 18 ++++++
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
 drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           |  6 +-
 drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
 drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
 7 files changed, 110 insertions(+), 5 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH 0/2] i915: CAGF and RC6 changes for MTL

[Badal Nilawar]

This series includes the code changes to get CAGF, RC State and 
C6 Residency of MTL.

v2: Included "Use GEN12 RPSTAT register" patch 

v3: 
  - Rebased
  - Dropped "Use GEN12 RPSTAT register" patch from this series
    going to send separate series for it

Badal Nilawar (2):
  drm/i915/mtl: Modify CAGF functions for MTL
  drm/i915/mtl: Add C6 residency support for MTL SAMedia

 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 18 ++++++
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
 drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           |  6 +-
 drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
 drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
 7 files changed, 110 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Badal Nilawar]

Updated the CAGF functions to get actual resolved frequency of
3D and SAMedia

Bspec: 66300

Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
 drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 2275ee47da95..7819d32db956 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1510,6 +1510,14 @@
 #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
 #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
 
+/*
+ * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
+ * 3D - 0x0C60 , SAMedia - 0x380C60
+ * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
+ */
+#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
+#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
+
 #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
 #define   GEN11_CSME				(31)
 #define   GEN11_GUNIT				(28)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 17b40b625e31..c2349949ebae 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
 
 	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
 		cagf = (rpstat >> 8) & 0xff;
+	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+		cagf = rpstat & MTL_CAGF_MASK;
 	else if (GRAPHICS_VER(i915) >= 9)
 		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
 	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
@@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
 		vlv_punit_get(i915);
 		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
 		vlv_punit_put(i915);
-	} else if (GRAPHICS_VER(i915) >= 6) {
+	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
+	else if (GRAPHICS_VER(i915) >= 6) {
 		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
 	} else {
 		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
-- 
2.25.1

[Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Badal Nilawar]

Updated the CAGF functions to get actual resolved frequency of
3D and SAMedia

Bspec: 66300

Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
 drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 2275ee47da95..7819d32db956 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1510,6 +1510,14 @@
 #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
 #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
 
+/*
+ * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
+ * 3D - 0x0C60 , SAMedia - 0x380C60
+ * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
+ */
+#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
+#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
+
 #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
 #define   GEN11_CSME				(31)
 #define   GEN11_GUNIT				(28)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 17b40b625e31..c2349949ebae 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
 
 	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
 		cagf = (rpstat >> 8) & 0xff;
+	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+		cagf = rpstat & MTL_CAGF_MASK;
 	else if (GRAPHICS_VER(i915) >= 9)
 		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
 	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
@@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
 		vlv_punit_get(i915);
 		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
 		vlv_punit_put(i915);
-	} else if (GRAPHICS_VER(i915) >= 6) {
+	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
+	else if (GRAPHICS_VER(i915) >= 6) {
 		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
 	} else {
 		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
-- 
2.25.1

[PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Badal Nilawar]

For MTL SAMedia updated relevant functions and places in the code to get
Media C6 residency.

v2: Fixed review comments (Ashutosh)

Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Chris Wilson <chris.p.wilson@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
 drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
 drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
 drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
 6 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 68310881a793..053167b506a9 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
 	return 0;
 }
 
+static int mtl_drpc(struct seq_file *m)
+{
+	struct intel_gt *gt = m->private;
+	struct intel_uncore *uncore = gt->uncore;
+	u32 gt_core_status, rcctl1, global_forcewake;
+	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
+	i915_reg_t reg;
+
+	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
+
+	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
+
+	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
+	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
+	mtl_powergate_status = intel_uncore_read(uncore,
+						 GEN9_PWRGT_DOMAIN_STATUS);
+
+	seq_printf(m, "RC6 Enabled: %s\n",
+		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
+	if (gt->type == GT_MEDIA) {
+		seq_printf(m, "Media Well Gating Enabled: %s\n",
+			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
+	} else {
+		seq_printf(m, "Render Well Gating Enabled: %s\n",
+			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
+	}
+
+	seq_puts(m, "Current RC state: ");
+
+	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
+	case MTL_CC0:
+		seq_puts(m, "on\n");
+		break;
+	case MTL_CC6:
+		seq_puts(m, "RC6\n");
+		break;
+	default:
+		seq_puts(m, "Unknown\n");
+		break;
+	}
+
+	if (gt->type == GT_MEDIA)
+		seq_printf(m, "Media Power Well: %s\n",
+			   (mtl_powergate_status &
+			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
+	else
+		seq_printf(m, "Render Power Well: %s\n",
+			   (mtl_powergate_status &
+			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
+
+	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
+	print_rc6_res(m, "RC6 residency since boot:", reg);
+
+	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
+
+	return fw_domains_show(m, NULL);
+}
+
 static int drpc_show(struct seq_file *m, void *unused)
 {
 	struct intel_gt *gt = m->private;
@@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
 	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
 		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
 			err = vlv_drpc(m);
+		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+			err = mtl_drpc(m);
 		else if (GRAPHICS_VER(i915) >= 6)
 			err = gen6_drpc(m);
 		else
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 7819d32db956..8a56fd873228 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1517,6 +1517,16 @@
  */
 #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
 #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
+#define   MTL_CC0                      0x0
+#define   MTL_CC6                      0x3
+#define   MTL_CC_SHIFT                 9
+#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
+
+/*
+ * MTL: This register contains the total MC6 residency time that SAMedia was
+ * since boot
+ */
+#define MTL_MEDIA_MC6                          _MMIO(0x138048)
 
 #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
 #define   GEN11_CSME				(31)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index 54deae45d81f..7ab1d776673a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
 
 static u32 __rc6_residency_ms_show(struct intel_gt *gt)
 {
-	return get_residency(gt, GEN6_GT_GFX_RC6);
+	i915_reg_t reg;
+
+	if (gt->type == GT_MEDIA)
+		reg = MTL_MEDIA_MC6;
+	else
+		reg = GEN6_GT_GFX_RC6;
+
+	return get_residency(gt, reg);
 }
 
 static ssize_t rc6_residency_ms_show(struct device *dev,
diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
index f8d0523f4c18..26f71f7f07c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
 	unsigned long flags;
 	unsigned int i;
 	u32 mul, div;
+	i915_reg_t base;
 
 	if (!rc6->supported)
 		return 0;
@@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
 	 * other so we can use the relative address, compared to the smallest
 	 * one as the index into driver storage.
 	 */
+	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
+	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
 	i = (i915_mmio_reg_offset(reg) -
-	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
+	     i915_mmio_reg_offset(base)) / sizeof(u32);
 	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
 		return 0;
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 8c70b7e12074..28c6a4b6b8d1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -15,11 +15,18 @@
 
 static u64 rc6_residency(struct intel_rc6 *rc6)
 {
+	struct intel_gt *gt = rc6_to_gt(rc6);
+	i915_reg_t reg;
 	u64 result;
 
 	/* XXX VLV_GT_MEDIA_RC6? */
 
-	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
+	if (gt->type == GT_MEDIA)
+		reg = MTL_MEDIA_MC6;
+	else
+		reg = GEN6_GT_GFX_RC6;
+
+	result = intel_rc6_residency_ns(rc6, reg);
 	if (HAS_RC6p(rc6_to_i915(rc6)))
 		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
 	if (HAS_RC6pp(rc6_to_i915(rc6)))
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..6ec139668641 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
 static u64 __get_rc6(struct intel_gt *gt)
 {
 	struct drm_i915_private *i915 = gt->i915;
+	i915_reg_t reg;
 	u64 val;
 
-	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
+	if (gt->type == GT_MEDIA)
+		reg = MTL_MEDIA_MC6;
+	else
+		reg = GEN6_GT_GFX_RC6;
+
+	val = intel_rc6_residency_ns(&gt->rc6, reg);
 
 	if (HAS_RC6p(i915))
 		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);
-- 
2.25.1

[Intel-gfx] [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Badal Nilawar]

For MTL SAMedia updated relevant functions and places in the code to get
Media C6 residency.

v2: Fixed review comments (Ashutosh)

Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Chris Wilson <chris.p.wilson@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
 drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
 drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
 drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
 6 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 68310881a793..053167b506a9 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
 	return 0;
 }
 
+static int mtl_drpc(struct seq_file *m)
+{
+	struct intel_gt *gt = m->private;
+	struct intel_uncore *uncore = gt->uncore;
+	u32 gt_core_status, rcctl1, global_forcewake;
+	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
+	i915_reg_t reg;
+
+	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
+
+	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
+
+	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
+	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
+	mtl_powergate_status = intel_uncore_read(uncore,
+						 GEN9_PWRGT_DOMAIN_STATUS);
+
+	seq_printf(m, "RC6 Enabled: %s\n",
+		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
+	if (gt->type == GT_MEDIA) {
+		seq_printf(m, "Media Well Gating Enabled: %s\n",
+			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
+	} else {
+		seq_printf(m, "Render Well Gating Enabled: %s\n",
+			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
+	}
+
+	seq_puts(m, "Current RC state: ");
+
+	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
+	case MTL_CC0:
+		seq_puts(m, "on\n");
+		break;
+	case MTL_CC6:
+		seq_puts(m, "RC6\n");
+		break;
+	default:
+		seq_puts(m, "Unknown\n");
+		break;
+	}
+
+	if (gt->type == GT_MEDIA)
+		seq_printf(m, "Media Power Well: %s\n",
+			   (mtl_powergate_status &
+			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
+	else
+		seq_printf(m, "Render Power Well: %s\n",
+			   (mtl_powergate_status &
+			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
+
+	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
+	print_rc6_res(m, "RC6 residency since boot:", reg);
+
+	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
+
+	return fw_domains_show(m, NULL);
+}
+
 static int drpc_show(struct seq_file *m, void *unused)
 {
 	struct intel_gt *gt = m->private;
@@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
 	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
 		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
 			err = vlv_drpc(m);
+		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+			err = mtl_drpc(m);
 		else if (GRAPHICS_VER(i915) >= 6)
 			err = gen6_drpc(m);
 		else
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 7819d32db956..8a56fd873228 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1517,6 +1517,16 @@
  */
 #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
 #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
+#define   MTL_CC0                      0x0
+#define   MTL_CC6                      0x3
+#define   MTL_CC_SHIFT                 9
+#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
+
+/*
+ * MTL: This register contains the total MC6 residency time that SAMedia was
+ * since boot
+ */
+#define MTL_MEDIA_MC6                          _MMIO(0x138048)
 
 #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
 #define   GEN11_CSME				(31)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index 54deae45d81f..7ab1d776673a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
 
 static u32 __rc6_residency_ms_show(struct intel_gt *gt)
 {
-	return get_residency(gt, GEN6_GT_GFX_RC6);
+	i915_reg_t reg;
+
+	if (gt->type == GT_MEDIA)
+		reg = MTL_MEDIA_MC6;
+	else
+		reg = GEN6_GT_GFX_RC6;
+
+	return get_residency(gt, reg);
 }
 
 static ssize_t rc6_residency_ms_show(struct device *dev,
diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
index f8d0523f4c18..26f71f7f07c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
 	unsigned long flags;
 	unsigned int i;
 	u32 mul, div;
+	i915_reg_t base;
 
 	if (!rc6->supported)
 		return 0;
@@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
 	 * other so we can use the relative address, compared to the smallest
 	 * one as the index into driver storage.
 	 */
+	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
+	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
 	i = (i915_mmio_reg_offset(reg) -
-	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
+	     i915_mmio_reg_offset(base)) / sizeof(u32);
 	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
 		return 0;
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
index 8c70b7e12074..28c6a4b6b8d1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
@@ -15,11 +15,18 @@
 
 static u64 rc6_residency(struct intel_rc6 *rc6)
 {
+	struct intel_gt *gt = rc6_to_gt(rc6);
+	i915_reg_t reg;
 	u64 result;
 
 	/* XXX VLV_GT_MEDIA_RC6? */
 
-	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
+	if (gt->type == GT_MEDIA)
+		reg = MTL_MEDIA_MC6;
+	else
+		reg = GEN6_GT_GFX_RC6;
+
+	result = intel_rc6_residency_ns(rc6, reg);
 	if (HAS_RC6p(rc6_to_i915(rc6)))
 		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
 	if (HAS_RC6pp(rc6_to_i915(rc6)))
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..6ec139668641 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
 static u64 __get_rc6(struct intel_gt *gt)
 {
 	struct drm_i915_private *i915 = gt->i915;
+	i915_reg_t reg;
 	u64 val;
 
-	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
+	if (gt->type == GT_MEDIA)
+		reg = MTL_MEDIA_MC6;
+	else
+		reg = GEN6_GT_GFX_RC6;
+
+	val = intel_rc6_residency_ns(&gt->rc6, reg);
 
 	if (HAS_RC6p(i915))
 		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);
-- 
2.25.1

Re: [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Jani Nikula]

On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> For MTL SAMedia updated relevant functions and places in the code to get
> Media C6 residency.
>
> v2: Fixed review comments (Ashutosh)
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Chris Wilson <chris.p.wilson@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
>  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
>  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
>  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
>  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
>  6 files changed, 97 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> index 68310881a793..053167b506a9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
>  	return 0;
>  }
>  
> +static int mtl_drpc(struct seq_file *m)
> +{
> +	struct intel_gt *gt = m->private;
> +	struct intel_uncore *uncore = gt->uncore;
> +	u32 gt_core_status, rcctl1, global_forcewake;
> +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> +	i915_reg_t reg;
> +
> +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> +
> +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
> +
> +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> +	mtl_powergate_status = intel_uncore_read(uncore,
> +						 GEN9_PWRGT_DOMAIN_STATUS);
> +
> +	seq_printf(m, "RC6 Enabled: %s\n",
> +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> +	if (gt->type == GT_MEDIA) {
> +		seq_printf(m, "Media Well Gating Enabled: %s\n",
> +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> +	} else {
> +		seq_printf(m, "Render Well Gating Enabled: %s\n",
> +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> +	}
> +
> +	seq_puts(m, "Current RC state: ");
> +
> +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
> +	case MTL_CC0:
> +		seq_puts(m, "on\n");
> +		break;
> +	case MTL_CC6:
> +		seq_puts(m, "RC6\n");
> +		break;
> +	default:
> +		seq_puts(m, "Unknown\n");
> +		break;
> +	}
> +
> +	if (gt->type == GT_MEDIA)
> +		seq_printf(m, "Media Power Well: %s\n",
> +			   (mtl_powergate_status &
> +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
> +	else
> +		seq_printf(m, "Render Power Well: %s\n",
> +			   (mtl_powergate_status &
> +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> +
> +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
> +	print_rc6_res(m, "RC6 residency since boot:", reg);

Cc: Tvrtko, Joonas, Rodrigo

IMO the register is not a good abstraction to build interfaces on. I see
that this is not where the idea is introduced, but it'll probably get
you in trouble later on.

BR,
Jani.

> +
> +	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
> +
> +	return fw_domains_show(m, NULL);
> +}
> +
>  static int drpc_show(struct seq_file *m, void *unused)
>  {
>  	struct intel_gt *gt = m->private;
> @@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
>  	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
>  		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  			err = vlv_drpc(m);
> +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +			err = mtl_drpc(m);
>  		else if (GRAPHICS_VER(i915) >= 6)
>  			err = gen6_drpc(m);
>  		else
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 7819d32db956..8a56fd873228 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1517,6 +1517,16 @@
>   */
>  #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
>  #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> +#define   MTL_CC0                      0x0
> +#define   MTL_CC6                      0x3
> +#define   MTL_CC_SHIFT                 9
> +#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
> +
> +/*
> + * MTL: This register contains the total MC6 residency time that SAMedia was
> + * since boot
> + */
> +#define MTL_MEDIA_MC6                          _MMIO(0x138048)
>  
>  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>  #define   GEN11_CSME				(31)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 54deae45d81f..7ab1d776673a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
>  
>  static u32 __rc6_residency_ms_show(struct intel_gt *gt)
>  {
> -	return get_residency(gt, GEN6_GT_GFX_RC6);
> +	i915_reg_t reg;
> +
> +	if (gt->type == GT_MEDIA)
> +		reg = MTL_MEDIA_MC6;
> +	else
> +		reg = GEN6_GT_GFX_RC6;
> +
> +	return get_residency(gt, reg);
>  }
>  
>  static ssize_t rc6_residency_ms_show(struct device *dev,
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index f8d0523f4c18..26f71f7f07c6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>  	unsigned long flags;
>  	unsigned int i;
>  	u32 mul, div;
> +	i915_reg_t base;
>  
>  	if (!rc6->supported)
>  		return 0;
> @@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>  	 * other so we can use the relative address, compared to the smallest
>  	 * one as the index into driver storage.
>  	 */
> +	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
> +	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
>  	i = (i915_mmio_reg_offset(reg) -
> -	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
> +	     i915_mmio_reg_offset(base)) / sizeof(u32);
>  	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
>  		return 0;
>  
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index 8c70b7e12074..28c6a4b6b8d1 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -15,11 +15,18 @@
>  
>  static u64 rc6_residency(struct intel_rc6 *rc6)
>  {
> +	struct intel_gt *gt = rc6_to_gt(rc6);
> +	i915_reg_t reg;
>  	u64 result;
>  
>  	/* XXX VLV_GT_MEDIA_RC6? */
>  
> -	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
> +	if (gt->type == GT_MEDIA)
> +		reg = MTL_MEDIA_MC6;
> +	else
> +		reg = GEN6_GT_GFX_RC6;
> +
> +	result = intel_rc6_residency_ns(rc6, reg);
>  	if (HAS_RC6p(rc6_to_i915(rc6)))
>  		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
>  	if (HAS_RC6pp(rc6_to_i915(rc6)))
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 958b37123bf1..6ec139668641 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>  static u64 __get_rc6(struct intel_gt *gt)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
> +	i915_reg_t reg;
>  	u64 val;
>  
> -	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
> +	if (gt->type == GT_MEDIA)
> +		reg = MTL_MEDIA_MC6;
> +	else
> +		reg = GEN6_GT_GFX_RC6;
> +
> +	val = intel_rc6_residency_ns(&gt->rc6, reg);
>  
>  	if (HAS_RC6p(i915))
>  		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Jani Nikula]

On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> For MTL SAMedia updated relevant functions and places in the code to get
> Media C6 residency.
>
> v2: Fixed review comments (Ashutosh)
>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Chris Wilson <chris.p.wilson@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
>  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
>  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
>  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
>  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
>  6 files changed, 97 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> index 68310881a793..053167b506a9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
>  	return 0;
>  }
>  
> +static int mtl_drpc(struct seq_file *m)
> +{
> +	struct intel_gt *gt = m->private;
> +	struct intel_uncore *uncore = gt->uncore;
> +	u32 gt_core_status, rcctl1, global_forcewake;
> +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> +	i915_reg_t reg;
> +
> +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> +
> +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
> +
> +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> +	mtl_powergate_status = intel_uncore_read(uncore,
> +						 GEN9_PWRGT_DOMAIN_STATUS);
> +
> +	seq_printf(m, "RC6 Enabled: %s\n",
> +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> +	if (gt->type == GT_MEDIA) {
> +		seq_printf(m, "Media Well Gating Enabled: %s\n",
> +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> +	} else {
> +		seq_printf(m, "Render Well Gating Enabled: %s\n",
> +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> +	}
> +
> +	seq_puts(m, "Current RC state: ");
> +
> +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
> +	case MTL_CC0:
> +		seq_puts(m, "on\n");
> +		break;
> +	case MTL_CC6:
> +		seq_puts(m, "RC6\n");
> +		break;
> +	default:
> +		seq_puts(m, "Unknown\n");
> +		break;
> +	}
> +
> +	if (gt->type == GT_MEDIA)
> +		seq_printf(m, "Media Power Well: %s\n",
> +			   (mtl_powergate_status &
> +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
> +	else
> +		seq_printf(m, "Render Power Well: %s\n",
> +			   (mtl_powergate_status &
> +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> +
> +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
> +	print_rc6_res(m, "RC6 residency since boot:", reg);

Cc: Tvrtko, Joonas, Rodrigo

IMO the register is not a good abstraction to build interfaces on. I see
that this is not where the idea is introduced, but it'll probably get
you in trouble later on.

BR,
Jani.

> +
> +	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
> +
> +	return fw_domains_show(m, NULL);
> +}
> +
>  static int drpc_show(struct seq_file *m, void *unused)
>  {
>  	struct intel_gt *gt = m->private;
> @@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
>  	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
>  		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  			err = vlv_drpc(m);
> +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +			err = mtl_drpc(m);
>  		else if (GRAPHICS_VER(i915) >= 6)
>  			err = gen6_drpc(m);
>  		else
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 7819d32db956..8a56fd873228 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1517,6 +1517,16 @@
>   */
>  #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
>  #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> +#define   MTL_CC0                      0x0
> +#define   MTL_CC6                      0x3
> +#define   MTL_CC_SHIFT                 9
> +#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
> +
> +/*
> + * MTL: This register contains the total MC6 residency time that SAMedia was
> + * since boot
> + */
> +#define MTL_MEDIA_MC6                          _MMIO(0x138048)
>  
>  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>  #define   GEN11_CSME				(31)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 54deae45d81f..7ab1d776673a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
>  
>  static u32 __rc6_residency_ms_show(struct intel_gt *gt)
>  {
> -	return get_residency(gt, GEN6_GT_GFX_RC6);
> +	i915_reg_t reg;
> +
> +	if (gt->type == GT_MEDIA)
> +		reg = MTL_MEDIA_MC6;
> +	else
> +		reg = GEN6_GT_GFX_RC6;
> +
> +	return get_residency(gt, reg);
>  }
>  
>  static ssize_t rc6_residency_ms_show(struct device *dev,
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index f8d0523f4c18..26f71f7f07c6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>  	unsigned long flags;
>  	unsigned int i;
>  	u32 mul, div;
> +	i915_reg_t base;
>  
>  	if (!rc6->supported)
>  		return 0;
> @@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>  	 * other so we can use the relative address, compared to the smallest
>  	 * one as the index into driver storage.
>  	 */
> +	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
> +	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
>  	i = (i915_mmio_reg_offset(reg) -
> -	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
> +	     i915_mmio_reg_offset(base)) / sizeof(u32);
>  	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
>  		return 0;
>  
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index 8c70b7e12074..28c6a4b6b8d1 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -15,11 +15,18 @@
>  
>  static u64 rc6_residency(struct intel_rc6 *rc6)
>  {
> +	struct intel_gt *gt = rc6_to_gt(rc6);
> +	i915_reg_t reg;
>  	u64 result;
>  
>  	/* XXX VLV_GT_MEDIA_RC6? */
>  
> -	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
> +	if (gt->type == GT_MEDIA)
> +		reg = MTL_MEDIA_MC6;
> +	else
> +		reg = GEN6_GT_GFX_RC6;
> +
> +	result = intel_rc6_residency_ns(rc6, reg);
>  	if (HAS_RC6p(rc6_to_i915(rc6)))
>  		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
>  	if (HAS_RC6pp(rc6_to_i915(rc6)))
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 958b37123bf1..6ec139668641 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>  static u64 __get_rc6(struct intel_gt *gt)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
> +	i915_reg_t reg;
>  	u64 val;
>  
> -	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
> +	if (gt->type == GT_MEDIA)
> +		reg = MTL_MEDIA_MC6;
> +	else
> +		reg = GEN6_GT_GFX_RC6;
> +
> +	val = intel_rc6_residency_ns(&gt->rc6, reg);
>  
>  	if (HAS_RC6p(i915))
>  		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] ✓ Fi.CI.BAT: success for i915: CAGF and RC6 changes for MTL (rev4)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4706 bytes --]

== Series Details ==

Series: i915: CAGF and RC6 changes for MTL (rev4)
URL   : https://patchwork.freedesktop.org/series/108156/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12154 -> Patchwork_108156v4
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/index.html

Participating hosts (44 -> 42)
------------------------------

  Additional (1): bat-dg2-11 
  Missing    (3): fi-ctg-p8600 fi-bdw-samus bat-jsl-1 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-pnv-d510:        NOTRUN -> [SKIP][1] ([fdo#109271])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/fi-pnv-d510/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
    - fi-bsw-kefka:       [PASS][2] -> [FAIL][3] ([i915#6298])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - {bat-adlm-1}:       [DMESG-WARN][4] ([i915#2867]) -> [PASS][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/bat-adlm-1/igt@gem_exec_suspend@basic-s3@smem.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/bat-adlm-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@requests:
    - fi-pnv-d510:        [DMESG-FAIL][6] ([i915#4528]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/fi-pnv-d510/igt@i915_selftest@live@requests.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/fi-pnv-d510/igt@i915_selftest@live@requests.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6850]: https://gitlab.freedesktop.org/drm/intel/issues/6850


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

  * Linux: CI_DRM_12154 -> Patchwork_108156v4

  CI-20190529: 20190529
  CI_DRM_12154: 3525875cb7903ed98c5b99d4f9d8b0b16b1d7f3f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6656: 24100c4e181c50e3678aeca9c641b8a43555ad73 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_108156v4: 3525875cb7903ed98c5b99d4f9d8b0b16b1d7f3f @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

43fd5a769d7e drm/i915/mtl: Add C6 residency support for MTL SAMedia
d7a628ed39de drm/i915/mtl: Modify CAGF functions for MTL

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/index.html

[-- Attachment #2: Type: text/html, Size: 3927 bytes --]

[Intel-gfx] ✗ Fi.CI.IGT: failure for i915: CAGF and RC6 changes for MTL (rev4)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 20656 bytes --]

== Series Details ==

Series: i915: CAGF and RC6 changes for MTL (rev4)
URL   : https://patchwork.freedesktop.org/series/108156/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12154_full -> Patchwork_108156v4_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_108156v4_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_108156v4_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (11 -> 10)
------------------------------

  Missing    (1): shard-rkl 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_108156v4_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_whisper@basic-queues-priority-all:
    - shard-iclb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb1/igt@gem_exec_whisper@basic-queues-priority-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb8/igt@gem_exec_whisper@basic-queues-priority-all.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@parallel-keep-in-fence:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([i915#4525]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb4/igt@gem_exec_balancer@parallel-keep-in-fence.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb3/igt@gem_exec_balancer@parallel-keep-in-fence.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2842]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-glk5/igt@gem_exec_fair@basic-none@vcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk8/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [PASS][7] -> [FAIL][8] ([i915#2842])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-tglb7/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-tglb8/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_flush@basic-uc-set-default:
    - shard-apl:          [PASS][9] -> [DMESG-FAIL][10] ([i915#6864])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl8/igt@gem_exec_flush@basic-uc-set-default.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl1/igt@gem_exec_flush@basic-uc-set-default.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([i915#5566] / [i915#716])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl8/igt@gen9_exec_parse@allowed-single.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl7/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([i915#3989] / [i915#454])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb1/igt@i915_pm_dc@dc6-psr.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb6/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([i915#180]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl1/igt@i915_suspend@sysfs-reader.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#3886])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl3/igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-bad-pixel-format-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][18] ([fdo#109271]) +15 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl3/igt@kms_ccs@pipe-d-bad-pixel-format-y_tiled_gen12_mc_ccs.html

  * igt@kms_chamelium@hdmi-cmp-planar-formats:
    - shard-apl:          NOTRUN -> [SKIP][19] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl3/igt@kms_chamelium@hdmi-cmp-planar-formats.html

  * igt@kms_cursor_crc@cursor-random-32x10:
    - shard-glk:          NOTRUN -> [SKIP][20] ([fdo#109271]) +6 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk9/igt@kms_cursor_crc@cursor-random-32x10.html

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
    - shard-apl:          [PASS][21] -> [FAIL][22] ([i915#2346])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl8/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][23] -> [FAIL][24] ([i915#2122])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-64bpp-4tile-downscaling@pipe-a-valid-mode:
    - shard-iclb:         NOTRUN -> [SKIP][25] ([i915#2587] / [i915#2672]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb5/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-64bpp-4tile-downscaling@pipe-a-valid-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-32bpp-4tile-downscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][26] ([i915#2672]) +3 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-32bpp-4tile-downscaling@pipe-a-default-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-linear-to-32bpp-linear-downscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][27] ([i915#3555])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-linear-to-32bpp-linear-downscaling@pipe-a-default-mode.html

  * igt@kms_psr@psr2_sprite_plane_onoff:
    - shard-iclb:         [PASS][28] -> [SKIP][29] ([fdo#109441]) +2 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb2/igt@kms_psr@psr2_sprite_plane_onoff.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb4/igt@kms_psr@psr2_sprite_plane_onoff.html

  * igt@kms_psr_stress_test@invalidate-primary-flip-overlay:
    - shard-iclb:         [PASS][30] -> [SKIP][31] ([i915#5519])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb4/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb3/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html

  * igt@sysfs_clients@create:
    - shard-glk:          NOTRUN -> [SKIP][32] ([fdo#109271] / [i915#2994])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk9/igt@sysfs_clients@create.html

  
#### Possible fixes ####

  * igt@gem_exec_balancer@parallel-keep-submit-fence:
    - shard-iclb:         [SKIP][33] ([i915#4525]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb3/igt@gem_exec_balancer@parallel-keep-submit-fence.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb2/igt@gem_exec_balancer@parallel-keep-submit-fence.html

  * igt@gem_exec_fair@basic-flow@rcs0:
    - shard-tglb:         [FAIL][35] ([i915#2842]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-tglb8/igt@gem_exec_fair@basic-flow@rcs0.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [FAIL][37] ([i915#2842]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl2/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         [FAIL][39] ([i915#2842]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb6/igt@gem_exec_fair@basic-throttle@rcs0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb6/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_flush@basic-uc-set-default:
    - shard-glk:          [DMESG-FAIL][41] ([i915#6864]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-glk3/igt@gem_exec_flush@basic-uc-set-default.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk9/igt@gem_exec_flush@basic-uc-set-default.html

  * igt@gem_huc_copy@huc-copy:
    - shard-tglb:         [SKIP][43] ([i915#2190]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-tglb7/igt@gem_huc_copy@huc-copy.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-tglb8/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_dc@dc5-psr:
    - shard-iclb:         [FAIL][45] ([i915#3989]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb3/igt@i915_pm_dc@dc5-psr.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb5/igt@i915_pm_dc@dc5-psr.html

  * igt@i915_pm_dc@dc9-dpms:
    - shard-iclb:         [SKIP][47] ([i915#4281]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb3/igt@i915_pm_dc@dc9-dpms.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb2/igt@i915_pm_dc@dc9-dpms.html

  * igt@kms_cursor_legacy@flip-vs-cursor@toggle:
    - shard-iclb:         [FAIL][49] ([i915#2346]) -> [PASS][50] +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes:
    - shard-apl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl3/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html

  * igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2:
    - shard-glk:          [DMESG-FAIL][53] ([i915#118] / [i915#1888]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-glk2/igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk2/igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         [SKIP][55] ([fdo#109441]) -> [PASS][56] +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_gtt.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_gtt.html

  
#### Warnings ####

  * igt@kms_plane_lowres@tiling-y@pipe-b-hdmi-a-1:
    - shard-glk:          [FAIL][57] -> [FAIL][58] ([i915#1888])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-glk2/igt@kms_plane_lowres@tiling-y@pipe-b-hdmi-a-1.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-glk2/igt@kms_plane_lowres@tiling-y@pipe-b-hdmi-a-1.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf:
    - shard-iclb:         [SKIP][59] ([i915#658]) -> [SKIP][60] ([i915#2920])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb3/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb2/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_sf@overlay-plane-update-continuous-sf:
    - shard-iclb:         [SKIP][61] ([i915#2920]) -> [SKIP][62] ([fdo#111068] / [i915#658])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb2/igt@kms_psr2_sf@overlay-plane-update-continuous-sf.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb5/igt@kms_psr2_sf@overlay-plane-update-continuous-sf.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area:
    - shard-iclb:         [SKIP][63] ([fdo#111068] / [i915#658]) -> [SKIP][64] ([i915#2920])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb3/igt@kms_psr2_sf@plane-move-sf-dmg-area.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb2/igt@kms_psr2_sf@plane-move-sf-dmg-area.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
    - shard-iclb:         [SKIP][65] ([i915#2920]) -> [SKIP][66] ([i915#658])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-iclb4/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html

  * igt@runner@aborted:
    - shard-apl:          ([FAIL][67], [FAIL][68], [FAIL][69], [FAIL][70], [FAIL][71]) ([i915#180] / [i915#3002] / [i915#4312] / [i915#5257] / [i915#6599]) -> ([FAIL][72], [FAIL][73], [FAIL][74], [FAIL][75], [FAIL][76], [FAIL][77], [FAIL][78]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312] / [i915#5257] / [i915#6599])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl1/igt@runner@aborted.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl8/igt@runner@aborted.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl1/igt@runner@aborted.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl3/igt@runner@aborted.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12154/shard-apl2/igt@runner@aborted.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl8/igt@runner@aborted.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl3/igt@runner@aborted.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl1/igt@runner@aborted.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl7/igt@runner@aborted.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl2/igt@runner@aborted.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl1/igt@runner@aborted.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_108156v4/shard-apl2/igt@runner@aborted.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1063]: https://gitlab.freedesktop.org/drm/intel/issues/1063
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3825]: https://gitlab.freedesktop.org/drm/intel/issues/3825
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5519]: https://gitlab.freedesktop.org/drm/intel/issues/5519
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6227]: https://gitlab.freedesktop.org/drm/intel/issues/6227
  [i915#6245]: https://gitlab.freedesktop.org/drm/intel/issues/6245
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6599]: https://gitlab.freedesktop.org/drm/intel/issues/6599
  [i915#6864]: https://gitlab.freedesktop.org/drm/intel/issues/6864
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716


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

  * Linux: CI_DRM_12154 -> Patchwork_108156v4

  CI-20190529: 20190529
  CI_DRM_12154: 3525875cb7903ed98c5b99d4f9d8b0b16b1d7f3f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6656: 24100c4e181c50e3678aeca9c641b8a43555ad73 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_108156v4: 3525875cb7903ed98c5b99d4f9d8b0b16b1d7f3f @ 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_108156v4/index.html

[-- Attachment #2: Type: text/html, Size: 22433 bytes --]

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Andi Shyti]

Hi Badal,

On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> Updated the CAGF functions to get actual resolved frequency of
> 3D and SAMedia

can you please use the imperative form? "Update" and not
"Updated".

Besides I don't really understand what you did from the
commit, can you please bea  bit more descriptive?

> Bspec: 66300
> 
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
>  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 2275ee47da95..7819d32db956 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1510,6 +1510,14 @@
>  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
>  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
>  
> +/*
> + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> + * 3D - 0x0C60 , SAMedia - 0x380C60
> + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> + */

This comment is not understandable... we don't have limits in
space, you can be a bit more explicit :)

Andi

> +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> +
>  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>  #define   GEN11_CSME				(31)
>  #define   GEN11_GUNIT				(28)
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 17b40b625e31..c2349949ebae 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
>  
>  	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  		cagf = (rpstat >> 8) & 0xff;
> +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +		cagf = rpstat & MTL_CAGF_MASK;
>  	else if (GRAPHICS_VER(i915) >= 9)
>  		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>  	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
>  		vlv_punit_get(i915);
>  		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
>  		vlv_punit_put(i915);
> -	} else if (GRAPHICS_VER(i915) >= 6) {
> +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
> +	else if (GRAPHICS_VER(i915) >= 6) {
>  		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
>  	} else {
>  		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> -- 
> 2.25.1

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Nilawar, Badal]

On 19-09-2022 22:19, Andi Shyti wrote:
> Hi Badal,
> 
> On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
>> Updated the CAGF functions to get actual resolved frequency of
>> 3D and SAMedia
> 
> can you please use the imperative form? "Update" and not
> "Updated".
Ok.
> 
> Besides I don't really understand what you did from the
> commit, can you please bea  bit more descriptive?
Sure I will describe more.
For MTL Current Actual GFX frequency (CAGF) can be obtained from regs 
0xc60 (GT0) and 0x380c60 (GT1). So to support MTL I modified functions 
read_cagf and intel_rps_get_cagf.
> 
>> Bspec: 66300
>>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
>>   drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> index 2275ee47da95..7819d32db956 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> @@ -1510,6 +1510,14 @@
>>   #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
>>   #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
>>   
>> +/*
>> + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
>> + * 3D - 0x0C60 , SAMedia - 0x380C60
>> + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
>> + */
> 
> This comment is not understandable... we don't have limits in
> space, you can be a bit more explicit :)
Below I defined only 0x0C60, so I am trying to say that 
intel_uncore_read/write functions takes care of adding GSI offset i.e. 
0x38000 if the access is for Gt1 (SAMEDIA).
This patch gives more clarity about GSI offset 
https://patchwork.freedesktop.org/patch/502004/?series=107908&rev=5

Regards,
Badal
> 
> Andi
> 
>> +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
>> +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
>> +
>>   #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>>   #define   GEN11_CSME				(31)
>>   #define   GEN11_GUNIT				(28)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
>> index 17b40b625e31..c2349949ebae 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
>> @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
>>   
>>   	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>>   		cagf = (rpstat >> 8) & 0xff;
>> +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> +		cagf = rpstat & MTL_CAGF_MASK;
>>   	else if (GRAPHICS_VER(i915) >= 9)
>>   		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>>   	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
>> @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
>>   		vlv_punit_get(i915);
>>   		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
>>   		vlv_punit_put(i915);
>> -	} else if (GRAPHICS_VER(i915) >= 6) {
>> +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
>> +	else if (GRAPHICS_VER(i915) >= 6) {
>>   		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
>>   	} else {
>>   		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
>> -- 
>> 2.25.1

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Nilawar, Badal]

On 19-09-2022 22:19, Andi Shyti wrote:
> Hi Badal,
> 
> On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
>> Updated the CAGF functions to get actual resolved frequency of
>> 3D and SAMedia
> 
> can you please use the imperative form? "Update" and not
> "Updated".
Ok.
> 
> Besides I don't really understand what you did from the
> commit, can you please bea  bit more descriptive?
Sure I will describe more.
For MTL Current Actual GFX frequency (CAGF) can be obtained from regs 
0xc60 (GT0) and 0x380c60 (GT1). So to support MTL I modified functions 
read_cagf and intel_rps_get_cagf.
> 
>> Bspec: 66300
>>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
>>   drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> index 2275ee47da95..7819d32db956 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> @@ -1510,6 +1510,14 @@
>>   #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
>>   #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
>>   
>> +/*
>> + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
>> + * 3D - 0x0C60 , SAMedia - 0x380C60
>> + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
>> + */
> 
> This comment is not understandable... we don't have limits in
> space, you can be a bit more explicit :)
Below I defined only 0x0C60, so I am trying to say that 
intel_uncore_read/write functions takes care of adding GSI offset i.e. 
0x38000 if the access is for Gt1 (SAMEDIA).
This patch gives more clarity about GSI offset 
https://patchwork.freedesktop.org/patch/502004/?series=107908&rev=5

Regards,
Badal
> 
> Andi
> 
>> +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
>> +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
>> +
>>   #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>>   #define   GEN11_CSME				(31)
>>   #define   GEN11_GUNIT				(28)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
>> index 17b40b625e31..c2349949ebae 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
>> @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
>>   
>>   	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>>   		cagf = (rpstat >> 8) & 0xff;
>> +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> +		cagf = rpstat & MTL_CAGF_MASK;
>>   	else if (GRAPHICS_VER(i915) >= 9)
>>   		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>>   	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
>> @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
>>   		vlv_punit_get(i915);
>>   		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
>>   		vlv_punit_put(i915);
>> -	} else if (GRAPHICS_VER(i915) >= 6) {
>> +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
>> +	else if (GRAPHICS_VER(i915) >= 6) {
>>   		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
>>   	} else {
>>   		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
>> -- 
>> 2.25.1

Re: [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Matt Roper]

On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> Updated the CAGF functions to get actual resolved frequency of
> 3D and SAMedia
> 
> Bspec: 66300
> 
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
>  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 2275ee47da95..7819d32db956 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1510,6 +1510,14 @@
>  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
>  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
>  
> +/*
> + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> + * 3D - 0x0C60 , SAMedia - 0x380C60
> + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> + */
> +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> +

This register is at the wrong place in the file (and is misformatted).
 - Keep it sorted with respect to the other registers in the file.
 - Write it as "0xc60" for consistency with all the other registers
   (i.e., lower-case hex, no unnecessary 0 prefix).
 - The whitespace between the name and the REG_GENMASK should be tabs,
   not spaces, ensuring it's lined up with the other definitions.

i915_reg.h turned into a huge mess over time because it wasn't
consistently organized or formatted so nobody knew what to do when
adding new registers.  We're trying to do a better job of following
consistent rules with the new register headers so that we don't wind up
with the same confusion again.

>  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>  #define   GEN11_CSME				(31)
>  #define   GEN11_GUNIT				(28)
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 17b40b625e31..c2349949ebae 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
>  
>  	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  		cagf = (rpstat >> 8) & 0xff;
> +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +		cagf = rpstat & MTL_CAGF_MASK;

Generally we try to put the newer platform at the top of if/else
ladders.  So this new MTL code should come before the VLV/CHV branch.

>  	else if (GRAPHICS_VER(i915) >= 9)
>  		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>  	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
>  		vlv_punit_get(i915);
>  		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
>  		vlv_punit_put(i915);
> -	} else if (GRAPHICS_VER(i915) >= 6) {
> +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);

Same here.


Matt

> +	else if (GRAPHICS_VER(i915) >= 6) {
>  		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
>  	} else {
>  		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Matt Roper]

On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> Updated the CAGF functions to get actual resolved frequency of
> 3D and SAMedia
> 
> Bspec: 66300
> 
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
>  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 2275ee47da95..7819d32db956 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1510,6 +1510,14 @@
>  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
>  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
>  
> +/*
> + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> + * 3D - 0x0C60 , SAMedia - 0x380C60
> + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> + */
> +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> +

This register is at the wrong place in the file (and is misformatted).
 - Keep it sorted with respect to the other registers in the file.
 - Write it as "0xc60" for consistency with all the other registers
   (i.e., lower-case hex, no unnecessary 0 prefix).
 - The whitespace between the name and the REG_GENMASK should be tabs,
   not spaces, ensuring it's lined up with the other definitions.

i915_reg.h turned into a huge mess over time because it wasn't
consistently organized or formatted so nobody knew what to do when
adding new registers.  We're trying to do a better job of following
consistent rules with the new register headers so that we don't wind up
with the same confusion again.

>  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>  #define   GEN11_CSME				(31)
>  #define   GEN11_GUNIT				(28)
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 17b40b625e31..c2349949ebae 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
>  
>  	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  		cagf = (rpstat >> 8) & 0xff;
> +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +		cagf = rpstat & MTL_CAGF_MASK;

Generally we try to put the newer platform at the top of if/else
ladders.  So this new MTL code should come before the VLV/CHV branch.

>  	else if (GRAPHICS_VER(i915) >= 9)
>  		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>  	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
>  		vlv_punit_get(i915);
>  		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
>  		vlv_punit_put(i915);
> -	} else if (GRAPHICS_VER(i915) >= 6) {
> +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);

Same here.


Matt

> +	else if (GRAPHICS_VER(i915) >= 6) {
>  		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
>  	} else {
>  		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation

Re: [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Matt Roper]

On Mon, Sep 19, 2022 at 03:46:47PM -0700, Matt Roper wrote:
> On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> > Updated the CAGF functions to get actual resolved frequency of
> > 3D and SAMedia
> > 
> > Bspec: 66300
> > 
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
> >  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 2275ee47da95..7819d32db956 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1510,6 +1510,14 @@
> >  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
> >  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
> >  
> > +/*
> > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> > + * 3D - 0x0C60 , SAMedia - 0x380C60
> > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> > + */

Also, this comment is unnecessary.  This is already how all GT registers
work so there's no reason to state this again on one one random
register.

> > +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> > +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> > +
> 
> This register is at the wrong place in the file (and is misformatted).
>  - Keep it sorted with respect to the other registers in the file.
>  - Write it as "0xc60" for consistency with all the other registers
>    (i.e., lower-case hex, no unnecessary 0 prefix).
>  - The whitespace between the name and the REG_GENMASK should be tabs,
>    not spaces, ensuring it's lined up with the other definitions.
> 
> i915_reg.h turned into a huge mess over time because it wasn't
> consistently organized or formatted so nobody knew what to do when
> adding new registers.  We're trying to do a better job of following
> consistent rules with the new register headers so that we don't wind up
> with the same confusion again.
> 
> >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
> >  #define   GEN11_CSME				(31)
> >  #define   GEN11_GUNIT				(28)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > index 17b40b625e31..c2349949ebae 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
> >  
> >  	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >  		cagf = (rpstat >> 8) & 0xff;
> > +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > +		cagf = rpstat & MTL_CAGF_MASK;
> 
> Generally we try to put the newer platform at the top of if/else
> ladders.  So this new MTL code should come before the VLV/CHV branch.
> 
> >  	else if (GRAPHICS_VER(i915) >= 9)
> >  		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
> >  	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> > @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
> >  		vlv_punit_get(i915);
> >  		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> >  		vlv_punit_put(i915);
> > -	} else if (GRAPHICS_VER(i915) >= 6) {
> > +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
> 
> Same here.
> 
> 
> Matt
> 
> > +	else if (GRAPHICS_VER(i915) >= 6) {
> >  		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
> >  	} else {
> >  		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> > -- 
> > 2.25.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Matt Roper]

On Mon, Sep 19, 2022 at 03:46:47PM -0700, Matt Roper wrote:
> On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> > Updated the CAGF functions to get actual resolved frequency of
> > 3D and SAMedia
> > 
> > Bspec: 66300
> > 
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
> >  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 2275ee47da95..7819d32db956 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1510,6 +1510,14 @@
> >  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
> >  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
> >  
> > +/*
> > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> > + * 3D - 0x0C60 , SAMedia - 0x380C60
> > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> > + */

Also, this comment is unnecessary.  This is already how all GT registers
work so there's no reason to state this again on one one random
register.

> > +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> > +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> > +
> 
> This register is at the wrong place in the file (and is misformatted).
>  - Keep it sorted with respect to the other registers in the file.
>  - Write it as "0xc60" for consistency with all the other registers
>    (i.e., lower-case hex, no unnecessary 0 prefix).
>  - The whitespace between the name and the REG_GENMASK should be tabs,
>    not spaces, ensuring it's lined up with the other definitions.
> 
> i915_reg.h turned into a huge mess over time because it wasn't
> consistently organized or formatted so nobody knew what to do when
> adding new registers.  We're trying to do a better job of following
> consistent rules with the new register headers so that we don't wind up
> with the same confusion again.
> 
> >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
> >  #define   GEN11_CSME				(31)
> >  #define   GEN11_GUNIT				(28)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > index 17b40b625e31..c2349949ebae 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
> >  
> >  	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >  		cagf = (rpstat >> 8) & 0xff;
> > +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > +		cagf = rpstat & MTL_CAGF_MASK;
> 
> Generally we try to put the newer platform at the top of if/else
> ladders.  So this new MTL code should come before the VLV/CHV branch.
> 
> >  	else if (GRAPHICS_VER(i915) >= 9)
> >  		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
> >  	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> > @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
> >  		vlv_punit_get(i915);
> >  		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> >  		vlv_punit_put(i915);
> > -	} else if (GRAPHICS_VER(i915) >= 6) {
> > +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
> 
> Same here.
> 
> 
> Matt
> 
> > +	else if (GRAPHICS_VER(i915) >= 6) {
> >  		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
> >  	} else {
> >  		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> > -- 
> > 2.25.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation

Re: [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Dixit, Ashutosh]

On Mon, 19 Sep 2022 05:13:18 -0700, Jani Nikula wrote:
>
> On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> > For MTL SAMedia updated relevant functions and places in the code to get
> > Media C6 residency.
> >
> > v2: Fixed review comments (Ashutosh)
> >
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Cc: Chris Wilson <chris.p.wilson@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
> >  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
> >  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
> >  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
> >  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
> >  6 files changed, 97 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > index 68310881a793..053167b506a9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
> >	return 0;
> >  }
> >
> > +static int mtl_drpc(struct seq_file *m)
> > +{
> > +	struct intel_gt *gt = m->private;
> > +	struct intel_uncore *uncore = gt->uncore;
> > +	u32 gt_core_status, rcctl1, global_forcewake;
> > +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> > +	i915_reg_t reg;
> > +
> > +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> > +
> > +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
> > +
> > +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> > +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> > +	mtl_powergate_status = intel_uncore_read(uncore,
> > +						 GEN9_PWRGT_DOMAIN_STATUS);
> > +
> > +	seq_printf(m, "RC6 Enabled: %s\n",
> > +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> > +	if (gt->type == GT_MEDIA) {
> > +		seq_printf(m, "Media Well Gating Enabled: %s\n",
> > +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> > +	} else {
> > +		seq_printf(m, "Render Well Gating Enabled: %s\n",
> > +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> > +	}
> > +
> > +	seq_puts(m, "Current RC state: ");
> > +
> > +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
> > +	case MTL_CC0:
> > +		seq_puts(m, "on\n");
> > +		break;
> > +	case MTL_CC6:
> > +		seq_puts(m, "RC6\n");
> > +		break;
> > +	default:
> > +		seq_puts(m, "Unknown\n");
> > +		break;
> > +	}
> > +
> > +	if (gt->type == GT_MEDIA)
> > +		seq_printf(m, "Media Power Well: %s\n",
> > +			   (mtl_powergate_status &
> > +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
> > +	else
> > +		seq_printf(m, "Render Power Well: %s\n",
> > +			   (mtl_powergate_status &
> > +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> > +
> > +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
> > +	print_rc6_res(m, "RC6 residency since boot:", reg);
>
> Cc: Tvrtko, Joonas, Rodrigo
>

Hi Jani,

> IMO the register is not a good abstraction to build interfaces on. I see
> that this is not where the idea is introduced, but it'll probably get
> you in trouble later on.

By "this is not where the idea is introduced" are you referring to what we
did here:

https://patchwork.freedesktop.org/patch/502372/?series=108091&rev=5

in intel_gt_perf_limit_reasons_reg()?

Or, should we follow the schema of centralizing the register selection
depending on gt type in a single function here too (since this register
selection is repeated throughout this patch)?

Thanks.
--
Ashutosh



>
> BR,
> Jani.
>
> > +
> > +	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
> > +
> > +	return fw_domains_show(m, NULL);
> > +}
> > +
> >  static int drpc_show(struct seq_file *m, void *unused)
> >  {
> >	struct intel_gt *gt = m->private;
> > @@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
> >	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
> >		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >			err = vlv_drpc(m);
> > +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > +			err = mtl_drpc(m);
> >		else if (GRAPHICS_VER(i915) >= 6)
> >			err = gen6_drpc(m);
> >		else
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 7819d32db956..8a56fd873228 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1517,6 +1517,16 @@
> >   */
> >  #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> >  #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> > +#define   MTL_CC0                      0x0
> > +#define   MTL_CC6                      0x3
> > +#define   MTL_CC_SHIFT                 9
> > +#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
> > +
> > +/*
> > + * MTL: This register contains the total MC6 residency time that SAMedia was
> > + * since boot
> > + */
> > +#define MTL_MEDIA_MC6                          _MMIO(0x138048)
> >
> >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
> >  #define   GEN11_CSME				(31)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > index 54deae45d81f..7ab1d776673a 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > @@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
> >
> >  static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> >  {
> > -	return get_residency(gt, GEN6_GT_GFX_RC6);
> > +	i915_reg_t reg;
> > +
> > +	if (gt->type == GT_MEDIA)
> > +		reg = MTL_MEDIA_MC6;
> > +	else
> > +		reg = GEN6_GT_GFX_RC6;
> > +
> > +	return get_residency(gt, reg);
> >  }
> >
> >  static ssize_t rc6_residency_ms_show(struct device *dev,
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > index f8d0523f4c18..26f71f7f07c6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > @@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> >	unsigned long flags;
> >	unsigned int i;
> >	u32 mul, div;
> > +	i915_reg_t base;
> >
> >	if (!rc6->supported)
> >		return 0;
> > @@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> >	 * other so we can use the relative address, compared to the smallest
> >	 * one as the index into driver storage.
> >	 */
> > +	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
> > +	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
> >	i = (i915_mmio_reg_offset(reg) -
> > -	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
> > +	     i915_mmio_reg_offset(base)) / sizeof(u32);
> >	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
> >		return 0;
> >
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > index 8c70b7e12074..28c6a4b6b8d1 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > @@ -15,11 +15,18 @@
> >
> >  static u64 rc6_residency(struct intel_rc6 *rc6)
> >  {
> > +	struct intel_gt *gt = rc6_to_gt(rc6);
> > +	i915_reg_t reg;
> >	u64 result;
> >
> >	/* XXX VLV_GT_MEDIA_RC6? */
> >
> > -	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
> > +	if (gt->type == GT_MEDIA)
> > +		reg = MTL_MEDIA_MC6;
> > +	else
> > +		reg = GEN6_GT_GFX_RC6;
> > +
> > +	result = intel_rc6_residency_ns(rc6, reg);
> >	if (HAS_RC6p(rc6_to_i915(rc6)))
> >		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
> >	if (HAS_RC6pp(rc6_to_i915(rc6)))
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> > index 958b37123bf1..6ec139668641 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
> >  static u64 __get_rc6(struct intel_gt *gt)
> >  {
> >	struct drm_i915_private *i915 = gt->i915;
> > +	i915_reg_t reg;
> >	u64 val;
> >
> > -	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
> > +	if (gt->type == GT_MEDIA)
> > +		reg = MTL_MEDIA_MC6;
> > +	else
> > +		reg = GEN6_GT_GFX_RC6;
> > +
> > +	val = intel_rc6_residency_ns(&gt->rc6, reg);
> >
> >	if (HAS_RC6p(i915))
> >		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);
>
> --
> Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Dixit, Ashutosh]

On Mon, 19 Sep 2022 05:13:18 -0700, Jani Nikula wrote:
>
> On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> > For MTL SAMedia updated relevant functions and places in the code to get
> > Media C6 residency.
> >
> > v2: Fixed review comments (Ashutosh)
> >
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Cc: Chris Wilson <chris.p.wilson@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
> >  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
> >  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
> >  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
> >  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
> >  6 files changed, 97 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > index 68310881a793..053167b506a9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
> >	return 0;
> >  }
> >
> > +static int mtl_drpc(struct seq_file *m)
> > +{
> > +	struct intel_gt *gt = m->private;
> > +	struct intel_uncore *uncore = gt->uncore;
> > +	u32 gt_core_status, rcctl1, global_forcewake;
> > +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> > +	i915_reg_t reg;
> > +
> > +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> > +
> > +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
> > +
> > +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> > +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> > +	mtl_powergate_status = intel_uncore_read(uncore,
> > +						 GEN9_PWRGT_DOMAIN_STATUS);
> > +
> > +	seq_printf(m, "RC6 Enabled: %s\n",
> > +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> > +	if (gt->type == GT_MEDIA) {
> > +		seq_printf(m, "Media Well Gating Enabled: %s\n",
> > +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> > +	} else {
> > +		seq_printf(m, "Render Well Gating Enabled: %s\n",
> > +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> > +	}
> > +
> > +	seq_puts(m, "Current RC state: ");
> > +
> > +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
> > +	case MTL_CC0:
> > +		seq_puts(m, "on\n");
> > +		break;
> > +	case MTL_CC6:
> > +		seq_puts(m, "RC6\n");
> > +		break;
> > +	default:
> > +		seq_puts(m, "Unknown\n");
> > +		break;
> > +	}
> > +
> > +	if (gt->type == GT_MEDIA)
> > +		seq_printf(m, "Media Power Well: %s\n",
> > +			   (mtl_powergate_status &
> > +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
> > +	else
> > +		seq_printf(m, "Render Power Well: %s\n",
> > +			   (mtl_powergate_status &
> > +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> > +
> > +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
> > +	print_rc6_res(m, "RC6 residency since boot:", reg);
>
> Cc: Tvrtko, Joonas, Rodrigo
>

Hi Jani,

> IMO the register is not a good abstraction to build interfaces on. I see
> that this is not where the idea is introduced, but it'll probably get
> you in trouble later on.

By "this is not where the idea is introduced" are you referring to what we
did here:

https://patchwork.freedesktop.org/patch/502372/?series=108091&rev=5

in intel_gt_perf_limit_reasons_reg()?

Or, should we follow the schema of centralizing the register selection
depending on gt type in a single function here too (since this register
selection is repeated throughout this patch)?

Thanks.
--
Ashutosh



>
> BR,
> Jani.
>
> > +
> > +	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
> > +
> > +	return fw_domains_show(m, NULL);
> > +}
> > +
> >  static int drpc_show(struct seq_file *m, void *unused)
> >  {
> >	struct intel_gt *gt = m->private;
> > @@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
> >	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
> >		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >			err = vlv_drpc(m);
> > +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > +			err = mtl_drpc(m);
> >		else if (GRAPHICS_VER(i915) >= 6)
> >			err = gen6_drpc(m);
> >		else
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 7819d32db956..8a56fd873228 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1517,6 +1517,16 @@
> >   */
> >  #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> >  #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> > +#define   MTL_CC0                      0x0
> > +#define   MTL_CC6                      0x3
> > +#define   MTL_CC_SHIFT                 9
> > +#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
> > +
> > +/*
> > + * MTL: This register contains the total MC6 residency time that SAMedia was
> > + * since boot
> > + */
> > +#define MTL_MEDIA_MC6                          _MMIO(0x138048)
> >
> >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
> >  #define   GEN11_CSME				(31)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > index 54deae45d81f..7ab1d776673a 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > @@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
> >
> >  static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> >  {
> > -	return get_residency(gt, GEN6_GT_GFX_RC6);
> > +	i915_reg_t reg;
> > +
> > +	if (gt->type == GT_MEDIA)
> > +		reg = MTL_MEDIA_MC6;
> > +	else
> > +		reg = GEN6_GT_GFX_RC6;
> > +
> > +	return get_residency(gt, reg);
> >  }
> >
> >  static ssize_t rc6_residency_ms_show(struct device *dev,
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > index f8d0523f4c18..26f71f7f07c6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> > @@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> >	unsigned long flags;
> >	unsigned int i;
> >	u32 mul, div;
> > +	i915_reg_t base;
> >
> >	if (!rc6->supported)
> >		return 0;
> > @@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> >	 * other so we can use the relative address, compared to the smallest
> >	 * one as the index into driver storage.
> >	 */
> > +	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
> > +	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
> >	i = (i915_mmio_reg_offset(reg) -
> > -	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
> > +	     i915_mmio_reg_offset(base)) / sizeof(u32);
> >	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
> >		return 0;
> >
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > index 8c70b7e12074..28c6a4b6b8d1 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > @@ -15,11 +15,18 @@
> >
> >  static u64 rc6_residency(struct intel_rc6 *rc6)
> >  {
> > +	struct intel_gt *gt = rc6_to_gt(rc6);
> > +	i915_reg_t reg;
> >	u64 result;
> >
> >	/* XXX VLV_GT_MEDIA_RC6? */
> >
> > -	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
> > +	if (gt->type == GT_MEDIA)
> > +		reg = MTL_MEDIA_MC6;
> > +	else
> > +		reg = GEN6_GT_GFX_RC6;
> > +
> > +	result = intel_rc6_residency_ns(rc6, reg);
> >	if (HAS_RC6p(rc6_to_i915(rc6)))
> >		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
> >	if (HAS_RC6pp(rc6_to_i915(rc6)))
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> > index 958b37123bf1..6ec139668641 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
> >  static u64 __get_rc6(struct intel_gt *gt)
> >  {
> >	struct drm_i915_private *i915 = gt->i915;
> > +	i915_reg_t reg;
> >	u64 val;
> >
> > -	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
> > +	if (gt->type == GT_MEDIA)
> > +		reg = MTL_MEDIA_MC6;
> > +	else
> > +		reg = GEN6_GT_GFX_RC6;
> > +
> > +	val = intel_rc6_residency_ns(&gt->rc6, reg);
> >
> >	if (HAS_RC6p(i915))
> >		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);
>
> --
> Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Jani Nikula]

On Mon, 19 Sep 2022, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
> On Mon, 19 Sep 2022 05:13:18 -0700, Jani Nikula wrote:
>>
>> On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
>> > For MTL SAMedia updated relevant functions and places in the code to get
>> > Media C6 residency.
>> >
>> > v2: Fixed review comments (Ashutosh)
>> >
>> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
>> > Cc: Chris Wilson <chris.p.wilson@intel.com>
>> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
>> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
>> >  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
>> >  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
>> >  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
>> >  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
>> >  6 files changed, 97 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> > index 68310881a793..053167b506a9 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> > @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
>> >	return 0;
>> >  }
>> >
>> > +static int mtl_drpc(struct seq_file *m)
>> > +{
>> > +	struct intel_gt *gt = m->private;
>> > +	struct intel_uncore *uncore = gt->uncore;
>> > +	u32 gt_core_status, rcctl1, global_forcewake;
>> > +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
>> > +	i915_reg_t reg;
>> > +
>> > +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
>> > +
>> > +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
>> > +
>> > +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
>> > +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
>> > +	mtl_powergate_status = intel_uncore_read(uncore,
>> > +						 GEN9_PWRGT_DOMAIN_STATUS);
>> > +
>> > +	seq_printf(m, "RC6 Enabled: %s\n",
>> > +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
>> > +	if (gt->type == GT_MEDIA) {
>> > +		seq_printf(m, "Media Well Gating Enabled: %s\n",
>> > +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
>> > +	} else {
>> > +		seq_printf(m, "Render Well Gating Enabled: %s\n",
>> > +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
>> > +	}
>> > +
>> > +	seq_puts(m, "Current RC state: ");
>> > +
>> > +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
>> > +	case MTL_CC0:
>> > +		seq_puts(m, "on\n");
>> > +		break;
>> > +	case MTL_CC6:
>> > +		seq_puts(m, "RC6\n");
>> > +		break;
>> > +	default:
>> > +		seq_puts(m, "Unknown\n");
>> > +		break;
>> > +	}
>> > +
>> > +	if (gt->type == GT_MEDIA)
>> > +		seq_printf(m, "Media Power Well: %s\n",
>> > +			   (mtl_powergate_status &
>> > +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
>> > +	else
>> > +		seq_printf(m, "Render Power Well: %s\n",
>> > +			   (mtl_powergate_status &
>> > +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
>> > +
>> > +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
>> > +	print_rc6_res(m, "RC6 residency since boot:", reg);
>>
>> Cc: Tvrtko, Joonas, Rodrigo
>>
>
> Hi Jani,
>
>> IMO the register is not a good abstraction to build interfaces on. I see
>> that this is not where the idea is introduced, but it'll probably get
>> you in trouble later on.
>
> By "this is not where the idea is introduced" are you referring to what we
> did here:
>
> https://patchwork.freedesktop.org/patch/502372/?series=108091&rev=5
>
> in intel_gt_perf_limit_reasons_reg()?
>
> Or, should we follow the schema of centralizing the register selection
> depending on gt type in a single function here too (since this register
> selection is repeated throughout this patch)?

I'm looking at print_rc6_res(), for example.

It takes the register, reads it, and also passes the register around,
eventually to intel_rc6_residency_ns(). That does magic on the register
offset, so it assumes a certain multi-register layout, and relative from
GEN6_GT_GFX_RC6_LOCKED. Then it assumes the register contents are
different on different platforms.

So why did we pass around the register to begin with? The knowledge
about the register offsets and contents are spread around. What if
another platform gets added with a different register contents or layout
or offsets?

Registers are a really low level of abstraction, and IMO usually should
not be passed around like this.


BR,
Jani.


>
> Thanks.
> --
> Ashutosh
>
>
>
>>
>> BR,
>> Jani.
>>
>> > +
>> > +	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
>> > +
>> > +	return fw_domains_show(m, NULL);
>> > +}
>> > +
>> >  static int drpc_show(struct seq_file *m, void *unused)
>> >  {
>> >	struct intel_gt *gt = m->private;
>> > @@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
>> >	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
>> >		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>> >			err = vlv_drpc(m);
>> > +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> > +			err = mtl_drpc(m);
>> >		else if (GRAPHICS_VER(i915) >= 6)
>> >			err = gen6_drpc(m);
>> >		else
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > index 7819d32db956..8a56fd873228 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > @@ -1517,6 +1517,16 @@
>> >   */
>> >  #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
>> >  #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
>> > +#define   MTL_CC0                      0x0
>> > +#define   MTL_CC6                      0x3
>> > +#define   MTL_CC_SHIFT                 9
>> > +#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
>> > +
>> > +/*
>> > + * MTL: This register contains the total MC6 residency time that SAMedia was
>> > + * since boot
>> > + */
>> > +#define MTL_MEDIA_MC6                          _MMIO(0x138048)
>> >
>> >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>> >  #define   GEN11_CSME				(31)
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> > index 54deae45d81f..7ab1d776673a 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> > @@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
>> >
>> >  static u32 __rc6_residency_ms_show(struct intel_gt *gt)
>> >  {
>> > -	return get_residency(gt, GEN6_GT_GFX_RC6);
>> > +	i915_reg_t reg;
>> > +
>> > +	if (gt->type == GT_MEDIA)
>> > +		reg = MTL_MEDIA_MC6;
>> > +	else
>> > +		reg = GEN6_GT_GFX_RC6;
>> > +
>> > +	return get_residency(gt, reg);
>> >  }
>> >
>> >  static ssize_t rc6_residency_ms_show(struct device *dev,
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> > index f8d0523f4c18..26f71f7f07c6 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> > @@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>> >	unsigned long flags;
>> >	unsigned int i;
>> >	u32 mul, div;
>> > +	i915_reg_t base;
>> >
>> >	if (!rc6->supported)
>> >		return 0;
>> > @@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>> >	 * other so we can use the relative address, compared to the smallest
>> >	 * one as the index into driver storage.
>> >	 */
>> > +	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
>> > +	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
>> >	i = (i915_mmio_reg_offset(reg) -
>> > -	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
>> > +	     i915_mmio_reg_offset(base)) / sizeof(u32);
>> >	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
>> >		return 0;
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
>> > index 8c70b7e12074..28c6a4b6b8d1 100644
>> > --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
>> > +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
>> > @@ -15,11 +15,18 @@
>> >
>> >  static u64 rc6_residency(struct intel_rc6 *rc6)
>> >  {
>> > +	struct intel_gt *gt = rc6_to_gt(rc6);
>> > +	i915_reg_t reg;
>> >	u64 result;
>> >
>> >	/* XXX VLV_GT_MEDIA_RC6? */
>> >
>> > -	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
>> > +	if (gt->type == GT_MEDIA)
>> > +		reg = MTL_MEDIA_MC6;
>> > +	else
>> > +		reg = GEN6_GT_GFX_RC6;
>> > +
>> > +	result = intel_rc6_residency_ns(rc6, reg);
>> >	if (HAS_RC6p(rc6_to_i915(rc6)))
>> >		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
>> >	if (HAS_RC6pp(rc6_to_i915(rc6)))
>> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>> > index 958b37123bf1..6ec139668641 100644
>> > --- a/drivers/gpu/drm/i915/i915_pmu.c
>> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> > @@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>> >  static u64 __get_rc6(struct intel_gt *gt)
>> >  {
>> >	struct drm_i915_private *i915 = gt->i915;
>> > +	i915_reg_t reg;
>> >	u64 val;
>> >
>> > -	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
>> > +	if (gt->type == GT_MEDIA)
>> > +		reg = MTL_MEDIA_MC6;
>> > +	else
>> > +		reg = GEN6_GT_GFX_RC6;
>> > +
>> > +	val = intel_rc6_residency_ns(&gt->rc6, reg);
>> >
>> >	if (HAS_RC6p(i915))
>> >		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Jani Nikula]

On Mon, 19 Sep 2022, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
> On Mon, 19 Sep 2022 05:13:18 -0700, Jani Nikula wrote:
>>
>> On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
>> > For MTL SAMedia updated relevant functions and places in the code to get
>> > Media C6 residency.
>> >
>> > v2: Fixed review comments (Ashutosh)
>> >
>> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
>> > Cc: Chris Wilson <chris.p.wilson@intel.com>
>> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
>> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
>> >  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
>> >  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
>> >  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
>> >  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
>> >  6 files changed, 97 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> > index 68310881a793..053167b506a9 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> > @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
>> >	return 0;
>> >  }
>> >
>> > +static int mtl_drpc(struct seq_file *m)
>> > +{
>> > +	struct intel_gt *gt = m->private;
>> > +	struct intel_uncore *uncore = gt->uncore;
>> > +	u32 gt_core_status, rcctl1, global_forcewake;
>> > +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
>> > +	i915_reg_t reg;
>> > +
>> > +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
>> > +
>> > +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
>> > +
>> > +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
>> > +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
>> > +	mtl_powergate_status = intel_uncore_read(uncore,
>> > +						 GEN9_PWRGT_DOMAIN_STATUS);
>> > +
>> > +	seq_printf(m, "RC6 Enabled: %s\n",
>> > +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
>> > +	if (gt->type == GT_MEDIA) {
>> > +		seq_printf(m, "Media Well Gating Enabled: %s\n",
>> > +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
>> > +	} else {
>> > +		seq_printf(m, "Render Well Gating Enabled: %s\n",
>> > +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
>> > +	}
>> > +
>> > +	seq_puts(m, "Current RC state: ");
>> > +
>> > +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
>> > +	case MTL_CC0:
>> > +		seq_puts(m, "on\n");
>> > +		break;
>> > +	case MTL_CC6:
>> > +		seq_puts(m, "RC6\n");
>> > +		break;
>> > +	default:
>> > +		seq_puts(m, "Unknown\n");
>> > +		break;
>> > +	}
>> > +
>> > +	if (gt->type == GT_MEDIA)
>> > +		seq_printf(m, "Media Power Well: %s\n",
>> > +			   (mtl_powergate_status &
>> > +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
>> > +	else
>> > +		seq_printf(m, "Render Power Well: %s\n",
>> > +			   (mtl_powergate_status &
>> > +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
>> > +
>> > +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
>> > +	print_rc6_res(m, "RC6 residency since boot:", reg);
>>
>> Cc: Tvrtko, Joonas, Rodrigo
>>
>
> Hi Jani,
>
>> IMO the register is not a good abstraction to build interfaces on. I see
>> that this is not where the idea is introduced, but it'll probably get
>> you in trouble later on.
>
> By "this is not where the idea is introduced" are you referring to what we
> did here:
>
> https://patchwork.freedesktop.org/patch/502372/?series=108091&rev=5
>
> in intel_gt_perf_limit_reasons_reg()?
>
> Or, should we follow the schema of centralizing the register selection
> depending on gt type in a single function here too (since this register
> selection is repeated throughout this patch)?

I'm looking at print_rc6_res(), for example.

It takes the register, reads it, and also passes the register around,
eventually to intel_rc6_residency_ns(). That does magic on the register
offset, so it assumes a certain multi-register layout, and relative from
GEN6_GT_GFX_RC6_LOCKED. Then it assumes the register contents are
different on different platforms.

So why did we pass around the register to begin with? The knowledge
about the register offsets and contents are spread around. What if
another platform gets added with a different register contents or layout
or offsets?

Registers are a really low level of abstraction, and IMO usually should
not be passed around like this.


BR,
Jani.


>
> Thanks.
> --
> Ashutosh
>
>
>
>>
>> BR,
>> Jani.
>>
>> > +
>> > +	seq_printf(m, "Global Forcewake Requests: 0x%x\n", global_forcewake);
>> > +
>> > +	return fw_domains_show(m, NULL);
>> > +}
>> > +
>> >  static int drpc_show(struct seq_file *m, void *unused)
>> >  {
>> >	struct intel_gt *gt = m->private;
>> > @@ -279,6 +337,8 @@ static int drpc_show(struct seq_file *m, void *unused)
>> >	with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
>> >		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>> >			err = vlv_drpc(m);
>> > +		else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> > +			err = mtl_drpc(m);
>> >		else if (GRAPHICS_VER(i915) >= 6)
>> >			err = gen6_drpc(m);
>> >		else
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > index 7819d32db956..8a56fd873228 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > @@ -1517,6 +1517,16 @@
>> >   */
>> >  #define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
>> >  #define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
>> > +#define   MTL_CC0                      0x0
>> > +#define   MTL_CC6                      0x3
>> > +#define   MTL_CC_SHIFT                 9
>> > +#define   MTL_CC_MASK                  (0xf << MTL_CC_SHIFT)
>> > +
>> > +/*
>> > + * MTL: This register contains the total MC6 residency time that SAMedia was
>> > + * since boot
>> > + */
>> > +#define MTL_MEDIA_MC6                          _MMIO(0x138048)
>> >
>> >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>> >  #define   GEN11_CSME				(31)
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> > index 54deae45d81f..7ab1d776673a 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
>> > @@ -123,7 +123,14 @@ static ssize_t rc6_enable_show(struct device *dev,
>> >
>> >  static u32 __rc6_residency_ms_show(struct intel_gt *gt)
>> >  {
>> > -	return get_residency(gt, GEN6_GT_GFX_RC6);
>> > +	i915_reg_t reg;
>> > +
>> > +	if (gt->type == GT_MEDIA)
>> > +		reg = MTL_MEDIA_MC6;
>> > +	else
>> > +		reg = GEN6_GT_GFX_RC6;
>> > +
>> > +	return get_residency(gt, reg);
>> >  }
>> >
>> >  static ssize_t rc6_residency_ms_show(struct device *dev,
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> > index f8d0523f4c18..26f71f7f07c6 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> > @@ -745,6 +745,7 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>> >	unsigned long flags;
>> >	unsigned int i;
>> >	u32 mul, div;
>> > +	i915_reg_t base;
>> >
>> >	if (!rc6->supported)
>> >		return 0;
>> > @@ -756,8 +757,10 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
>> >	 * other so we can use the relative address, compared to the smallest
>> >	 * one as the index into driver storage.
>> >	 */
>> > +	base = (rc6_to_gt(rc6)->type == GT_MEDIA) ?
>> > +	       MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6_LOCKED;
>> >	i = (i915_mmio_reg_offset(reg) -
>> > -	     i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
>> > +	     i915_mmio_reg_offset(base)) / sizeof(u32);
>> >	if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
>> >		return 0;
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
>> > index 8c70b7e12074..28c6a4b6b8d1 100644
>> > --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
>> > +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
>> > @@ -15,11 +15,18 @@
>> >
>> >  static u64 rc6_residency(struct intel_rc6 *rc6)
>> >  {
>> > +	struct intel_gt *gt = rc6_to_gt(rc6);
>> > +	i915_reg_t reg;
>> >	u64 result;
>> >
>> >	/* XXX VLV_GT_MEDIA_RC6? */
>> >
>> > -	result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
>> > +	if (gt->type == GT_MEDIA)
>> > +		reg = MTL_MEDIA_MC6;
>> > +	else
>> > +		reg = GEN6_GT_GFX_RC6;
>> > +
>> > +	result = intel_rc6_residency_ns(rc6, reg);
>> >	if (HAS_RC6p(rc6_to_i915(rc6)))
>> >		result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
>> >	if (HAS_RC6pp(rc6_to_i915(rc6)))
>> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>> > index 958b37123bf1..6ec139668641 100644
>> > --- a/drivers/gpu/drm/i915/i915_pmu.c
>> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> > @@ -146,9 +146,15 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>> >  static u64 __get_rc6(struct intel_gt *gt)
>> >  {
>> >	struct drm_i915_private *i915 = gt->i915;
>> > +	i915_reg_t reg;
>> >	u64 val;
>> >
>> > -	val = intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6);
>> > +	if (gt->type == GT_MEDIA)
>> > +		reg = MTL_MEDIA_MC6;
>> > +	else
>> > +		reg = GEN6_GT_GFX_RC6;
>> > +
>> > +	val = intel_rc6_residency_ns(&gt->rc6, reg);
>> >
>> >	if (HAS_RC6p(i915))
>> >		val += intel_rc6_residency_ns(&gt->rc6, GEN6_GT_GFX_RC6p);
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Dixit, Ashutosh]

On Mon, 19 Sep 2022 15:49:17 -0700, Matt Roper wrote:
>
> On Mon, Sep 19, 2022 at 03:46:47PM -0700, Matt Roper wrote:
> > On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> > > Updated the CAGF functions to get actual resolved frequency of
> > > 3D and SAMedia
> > >
> > > Bspec: 66300
> > >
> > > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
> > >  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
> > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > index 2275ee47da95..7819d32db956 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > @@ -1510,6 +1510,14 @@
> > >  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
> > >  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
> > >
> > > +/*
> > > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> > > + * 3D - 0x0C60 , SAMedia - 0x380C60
> > > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> > > + */
>
> Also, this comment is unnecessary.  This is already how all GT registers
> work so there's no reason to state this again on one one random
> register.
>
> > > +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> > > +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> > > +
> >
> > This register is at the wrong place in the file (and is misformatted).
> >  - Keep it sorted with respect to the other registers in the file.
> >  - Write it as "0xc60" for consistency with all the other registers
> >    (i.e., lower-case hex, no unnecessary 0 prefix).
> >  - The whitespace between the name and the REG_GENMASK should be tabs,
> >    not spaces, ensuring it's lined up with the other definitions.
> >
> > i915_reg.h turned into a huge mess over time because it wasn't
> > consistently organized or formatted so nobody knew what to do when
> > adding new registers.  We're trying to do a better job of following
> > consistent rules with the new register headers so that we don't wind up
> > with the same confusion again.

Fixed in series version v5 (Patch version v2). Same for the comments below
too.

> >
> > >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
> > >  #define   GEN11_CSME				(31)
> > >  #define   GEN11_GUNIT				(28)
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > index 17b40b625e31..c2349949ebae 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
> > >
> > >	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> > >		cagf = (rpstat >> 8) & 0xff;
> > > +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > > +		cagf = rpstat & MTL_CAGF_MASK;
> >
> > Generally we try to put the newer platform at the top of if/else
> > ladders.  So this new MTL code should come before the VLV/CHV branch.

Done.

> >
> > >	else if (GRAPHICS_VER(i915) >= 9)
> > >		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
> > >	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> > > @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
> > >		vlv_punit_get(i915);
> > >		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> > >		vlv_punit_put(i915);
> > > -	} else if (GRAPHICS_VER(i915) >= 6) {
> > > +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > > +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
> >
> > Same here.

Done.

Thanks.
--
Ashutosh

> > > +	else if (GRAPHICS_VER(i915) >= 6) {
> > >		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
> > >	} else {
> > >		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> > > --
> > > 2.25.1
> > >
> >
> > --
> > Matt Roper
> > Graphics Software Engineer
> > VTT-OSGC Platform Enablement
> > Intel Corporation
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Dixit, Ashutosh]

On Mon, 19 Sep 2022 15:49:17 -0700, Matt Roper wrote:
>
> On Mon, Sep 19, 2022 at 03:46:47PM -0700, Matt Roper wrote:
> > On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> > > Updated the CAGF functions to get actual resolved frequency of
> > > 3D and SAMedia
> > >
> > > Bspec: 66300
> > >
> > > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
> > >  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
> > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > index 2275ee47da95..7819d32db956 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > @@ -1510,6 +1510,14 @@
> > >  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
> > >  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
> > >
> > > +/*
> > > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> > > + * 3D - 0x0C60 , SAMedia - 0x380C60
> > > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> > > + */
>
> Also, this comment is unnecessary.  This is already how all GT registers
> work so there's no reason to state this again on one one random
> register.
>
> > > +#define MTL_MIRROR_TARGET_WP1          _MMIO(0x0C60)
> > > +#define   MTL_CAGF_MASK                REG_GENMASK(8, 0)
> > > +
> >
> > This register is at the wrong place in the file (and is misformatted).
> >  - Keep it sorted with respect to the other registers in the file.
> >  - Write it as "0xc60" for consistency with all the other registers
> >    (i.e., lower-case hex, no unnecessary 0 prefix).
> >  - The whitespace between the name and the REG_GENMASK should be tabs,
> >    not spaces, ensuring it's lined up with the other definitions.
> >
> > i915_reg.h turned into a huge mess over time because it wasn't
> > consistently organized or formatted so nobody knew what to do when
> > adding new registers.  We're trying to do a better job of following
> > consistent rules with the new register headers so that we don't wind up
> > with the same confusion again.

Fixed in series version v5 (Patch version v2). Same for the comments below
too.

> >
> > >  #define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
> > >  #define   GEN11_CSME				(31)
> > >  #define   GEN11_GUNIT				(28)
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > index 17b40b625e31..c2349949ebae 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > @@ -2075,6 +2075,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
> > >
> > >	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> > >		cagf = (rpstat >> 8) & 0xff;
> > > +	else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > > +		cagf = rpstat & MTL_CAGF_MASK;
> >
> > Generally we try to put the newer platform at the top of if/else
> > ladders.  So this new MTL code should come before the VLV/CHV branch.

Done.

> >
> > >	else if (GRAPHICS_VER(i915) >= 9)
> > >		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
> > >	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> > > @@ -2098,7 +2100,9 @@ static u32 read_cagf(struct intel_rps *rps)
> > >		vlv_punit_get(i915);
> > >		freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> > >		vlv_punit_put(i915);
> > > -	} else if (GRAPHICS_VER(i915) >= 6) {
> > > +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> > > +		freq = intel_uncore_read(rps_to_gt(rps)->uncore, MTL_MIRROR_TARGET_WP1);
> >
> > Same here.

Done.

Thanks.
--
Ashutosh

> > > +	else if (GRAPHICS_VER(i915) >= 6) {
> > >		freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
> > >	} else {
> > >		freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> > > --
> > > 2.25.1
> > >
> >
> > --
> > Matt Roper
> > Graphics Software Engineer
> > VTT-OSGC Platform Enablement
> > Intel Corporation
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Dixit, Ashutosh]

On Mon, 19 Sep 2022 09:49:07 -0700, Andi Shyti wrote:
>
> Hi Badal,

Hi Andi,

Badal is out for a bit so I am sending out this version.

> On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> > Updated the CAGF functions to get actual resolved frequency of
> > 3D and SAMedia
>
> can you please use the imperative form? "Update" and not
> "Updated".

> Besides I don't really understand what you did from the
> commit, can you please bea  bit more descriptive?

Done in series version v5. Please take a look.

> > Bspec: 66300
> >
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
> >  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 2275ee47da95..7819d32db956 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1510,6 +1510,14 @@
> >  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
> >  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
> >
> > +/*
> > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> > + * 3D - 0x0C60 , SAMedia - 0x380C60
> > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> > + */
>
> This comment is not understandable... we don't have limits in
> space, you can be a bit more explicit :)

Based on Matt R's comment the comment has been deleted (except for the
first line). There is an explanation at the bottom of gt/intel_gt_regs.h.

Thanks.
--
Ashutosh

Re: [Intel-gfx] [PATCH 1/2] drm/i915/mtl: Modify CAGF functions for MTL

[Dixit, Ashutosh]

On Mon, 19 Sep 2022 09:49:07 -0700, Andi Shyti wrote:
>
> Hi Badal,

Hi Andi,

Badal is out for a bit so I am sending out this version.

> On Mon, Sep 19, 2022 at 05:29:05PM +0530, Badal Nilawar wrote:
> > Updated the CAGF functions to get actual resolved frequency of
> > 3D and SAMedia
>
> can you please use the imperative form? "Update" and not
> "Updated".

> Besides I don't really understand what you did from the
> commit, can you please bea  bit more descriptive?

Done in series version v5. Please take a look.

> > Bspec: 66300
> >
> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 ++++++++
> >  drivers/gpu/drm/i915/gt/intel_rps.c     | 6 +++++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 2275ee47da95..7819d32db956 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1510,6 +1510,14 @@
> >  #define VLV_RENDER_C0_COUNT			_MMIO(0x138118)
> >  #define VLV_MEDIA_C0_COUNT			_MMIO(0x13811c)
> >
> > +/*
> > + * MTL: Workpoint reg to get Core C state and act freq of 3D, SAMedia/
> > + * 3D - 0x0C60 , SAMedia - 0x380C60
> > + * Intel uncore handler redirects transactions for SAMedia to MTL_MEDIA_GSI_BASE
> > + */
>
> This comment is not understandable... we don't have limits in
> space, you can be a bit more explicit :)

Based on Matt R's comment the comment has been deleted (except for the
first line). There is an explanation at the bottom of gt/intel_gt_regs.h.

Thanks.
--
Ashutosh

Re: [Intel-gfx] [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Dixit, Ashutosh]

On Tue, 20 Sep 2022 01:06:52 -0700, Jani Nikula wrote:
>
> On Mon, 19 Sep 2022, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
> > On Mon, 19 Sep 2022 05:13:18 -0700, Jani Nikula wrote:
> >>
> >> On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> >> > For MTL SAMedia updated relevant functions and places in the code to get
> >> > Media C6 residency.
> >> >
> >> > v2: Fixed review comments (Ashutosh)
> >> >
> >> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> >> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> >> > Cc: Chris Wilson <chris.p.wilson@intel.com>
> >> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
> >> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
> >> >  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
> >> >  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
> >> >  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
> >> >  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
> >> >  6 files changed, 97 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> >> > index 68310881a793..053167b506a9 100644
> >> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> >> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> >> > @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
> >> >	return 0;
> >> >  }
> >> >
> >> > +static int mtl_drpc(struct seq_file *m)
> >> > +{
> >> > +	struct intel_gt *gt = m->private;
> >> > +	struct intel_uncore *uncore = gt->uncore;
> >> > +	u32 gt_core_status, rcctl1, global_forcewake;
> >> > +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> >> > +	i915_reg_t reg;
> >> > +
> >> > +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> >> > +
> >> > +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
> >> > +
> >> > +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> >> > +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> >> > +	mtl_powergate_status = intel_uncore_read(uncore,
> >> > +						 GEN9_PWRGT_DOMAIN_STATUS);
> >> > +
> >> > +	seq_printf(m, "RC6 Enabled: %s\n",
> >> > +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> >> > +	if (gt->type == GT_MEDIA) {
> >> > +		seq_printf(m, "Media Well Gating Enabled: %s\n",
> >> > +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> >> > +	} else {
> >> > +		seq_printf(m, "Render Well Gating Enabled: %s\n",
> >> > +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> >> > +	}
> >> > +
> >> > +	seq_puts(m, "Current RC state: ");
> >> > +
> >> > +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
> >> > +	case MTL_CC0:
> >> > +		seq_puts(m, "on\n");
> >> > +		break;
> >> > +	case MTL_CC6:
> >> > +		seq_puts(m, "RC6\n");
> >> > +		break;
> >> > +	default:
> >> > +		seq_puts(m, "Unknown\n");
> >> > +		break;
> >> > +	}
> >> > +
> >> > +	if (gt->type == GT_MEDIA)
> >> > +		seq_printf(m, "Media Power Well: %s\n",
> >> > +			   (mtl_powergate_status &
> >> > +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
> >> > +	else
> >> > +		seq_printf(m, "Render Power Well: %s\n",
> >> > +			   (mtl_powergate_status &
> >> > +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> >> > +
> >> > +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
> >> > +	print_rc6_res(m, "RC6 residency since boot:", reg);
> >>
> >> Cc: Tvrtko, Joonas, Rodrigo
> >>
> >
> > Hi Jani,
> >
> >> IMO the register is not a good abstraction to build interfaces on. I see
> >> that this is not where the idea is introduced, but it'll probably get
> >> you in trouble later on.
> >
> > By "this is not where the idea is introduced" are you referring to what we
> > did here:
> >
> > https://patchwork.freedesktop.org/patch/502372/?series=108091&rev=5
> >
> > in intel_gt_perf_limit_reasons_reg()?
> >
> > Or, should we follow the schema of centralizing the register selection
> > depending on gt type in a single function here too (since this register
> > selection is repeated throughout this patch)?
>
> I'm looking at print_rc6_res(), for example.
>
> It takes the register, reads it, and also passes the register around,
> eventually to intel_rc6_residency_ns(). That does magic on the register
> offset, so it assumes a certain multi-register layout, and relative from
> GEN6_GT_GFX_RC6_LOCKED. Then it assumes the register contents are
> different on different platforms.
>
> So why did we pass around the register to begin with? The knowledge
> about the register offsets and contents are spread around. What if
> another platform gets added with a different register contents or layout
> or offsets?
>
> Registers are a really low level of abstraction, and IMO usually should
> not be passed around like this.

Hi Jani, I've tried to fix this in v5 of this series based on some
discussion which I believe happened between Badal and Rodrigo:

https://patchwork.freedesktop.org/series/108156/

Please take a look at "drm/i915/gt: Change RC6 residency functions to
accept register ID's".

Thanks.
--
Ashutosh

Re: [PATCH 2/2] drm/i915/mtl: Add C6 residency support for MTL SAMedia

[Dixit, Ashutosh]

On Tue, 20 Sep 2022 01:06:52 -0700, Jani Nikula wrote:
>
> On Mon, 19 Sep 2022, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
> > On Mon, 19 Sep 2022 05:13:18 -0700, Jani Nikula wrote:
> >>
> >> On Mon, 19 Sep 2022, Badal Nilawar <badal.nilawar@intel.com> wrote:
> >> > For MTL SAMedia updated relevant functions and places in the code to get
> >> > Media C6 residency.
> >> >
> >> > v2: Fixed review comments (Ashutosh)
> >> >
> >> > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> >> > Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> >> > Cc: Chris Wilson <chris.p.wilson@intel.com>
> >> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 60 +++++++++++++++++++
> >> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h       | 10 ++++
> >> >  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   |  9 ++-
> >> >  drivers/gpu/drm/i915/gt/intel_rc6.c           |  5 +-
> >> >  drivers/gpu/drm/i915/gt/selftest_rc6.c        |  9 ++-
> >> >  drivers/gpu/drm/i915/i915_pmu.c               |  8 ++-
> >> >  6 files changed, 97 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> >> > index 68310881a793..053167b506a9 100644
> >> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> >> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> >> > @@ -269,6 +269,64 @@ static int ilk_drpc(struct seq_file *m)
> >> >	return 0;
> >> >  }
> >> >
> >> > +static int mtl_drpc(struct seq_file *m)
> >> > +{
> >> > +	struct intel_gt *gt = m->private;
> >> > +	struct intel_uncore *uncore = gt->uncore;
> >> > +	u32 gt_core_status, rcctl1, global_forcewake;
> >> > +	u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> >> > +	i915_reg_t reg;
> >> > +
> >> > +	gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> >> > +
> >> > +	global_forcewake = intel_uncore_read(uncore, FORCEWAKE_GT_GEN9);
> >> > +
> >> > +	rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> >> > +	mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> >> > +	mtl_powergate_status = intel_uncore_read(uncore,
> >> > +						 GEN9_PWRGT_DOMAIN_STATUS);
> >> > +
> >> > +	seq_printf(m, "RC6 Enabled: %s\n",
> >> > +		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> >> > +	if (gt->type == GT_MEDIA) {
> >> > +		seq_printf(m, "Media Well Gating Enabled: %s\n",
> >> > +			   str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> >> > +	} else {
> >> > +		seq_printf(m, "Render Well Gating Enabled: %s\n",
> >> > +			   str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> >> > +	}
> >> > +
> >> > +	seq_puts(m, "Current RC state: ");
> >> > +
> >> > +	switch ((gt_core_status & MTL_CC_MASK) >> MTL_CC_SHIFT) {
> >> > +	case MTL_CC0:
> >> > +		seq_puts(m, "on\n");
> >> > +		break;
> >> > +	case MTL_CC6:
> >> > +		seq_puts(m, "RC6\n");
> >> > +		break;
> >> > +	default:
> >> > +		seq_puts(m, "Unknown\n");
> >> > +		break;
> >> > +	}
> >> > +
> >> > +	if (gt->type == GT_MEDIA)
> >> > +		seq_printf(m, "Media Power Well: %s\n",
> >> > +			   (mtl_powergate_status &
> >> > +			    GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
> >> > +	else
> >> > +		seq_printf(m, "Render Power Well: %s\n",
> >> > +			   (mtl_powergate_status &
> >> > +			    GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> >> > +
> >> > +	reg = (gt->type == GT_MEDIA) ? MTL_MEDIA_MC6 : GEN6_GT_GFX_RC6;
> >> > +	print_rc6_res(m, "RC6 residency since boot:", reg);
> >>
> >> Cc: Tvrtko, Joonas, Rodrigo
> >>
> >
> > Hi Jani,
> >
> >> IMO the register is not a good abstraction to build interfaces on. I see
> >> that this is not where the idea is introduced, but it'll probably get
> >> you in trouble later on.
> >
> > By "this is not where the idea is introduced" are you referring to what we
> > did here:
> >
> > https://patchwork.freedesktop.org/patch/502372/?series=108091&rev=5
> >
> > in intel_gt_perf_limit_reasons_reg()?
> >
> > Or, should we follow the schema of centralizing the register selection
> > depending on gt type in a single function here too (since this register
> > selection is repeated throughout this patch)?
>
> I'm looking at print_rc6_res(), for example.
>
> It takes the register, reads it, and also passes the register around,
> eventually to intel_rc6_residency_ns(). That does magic on the register
> offset, so it assumes a certain multi-register layout, and relative from
> GEN6_GT_GFX_RC6_LOCKED. Then it assumes the register contents are
> different on different platforms.
>
> So why did we pass around the register to begin with? The knowledge
> about the register offsets and contents are spread around. What if
> another platform gets added with a different register contents or layout
> or offsets?
>
> Registers are a really low level of abstraction, and IMO usually should
> not be passed around like this.

Hi Jani, I've tried to fix this in v5 of this series based on some
discussion which I believe happened between Badal and Rodrigo:

https://patchwork.freedesktop.org/series/108156/

Please take a look at "drm/i915/gt: Change RC6 residency functions to
accept register ID's".

Thanks.
--
Ashutosh