[Intel-gfx] [PATCH v5 0/7] drm/i915: Media freq factor and per-gt enhancements/fixes

[Intel-gfx] [PATCH v5 0/7] drm/i915: Media freq factor and per-gt enhancements/fixes

[Ashutosh Dixit]

Some recent Intel dGfx platforms allow media IP to work at a different
frequency from the base GT. This patch series exposes sysfs controls for
this functionality in the new per-gt sysfs. Some enhancements and fixes to
previous per-gt functionality are also included to complete the new
functionality:
* Patches 1 and 2 implement basic sysfs controls for media freq
* Patch 3 extends previous pcode functions for multiple gt's
* Patch 4 inits pcode on different gt's
* Patch 5 adds a couple of pcode helpers
* Patch 6 uses the new pcode functions to retrieve media RP0/RPn freq
* Patch 7 fixes memory leaks in the previous per-gt sysfs implementation
  and some code refactoring

IGT tests for this new functionality have also been posted at:

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

Test-with: 20220426194111.5990-1-ashutosh.dixit@intel.com

v2: Fixed commit author on patches 5 and 6 (Rodrigo)
    Added new patch 4
v3: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
v4: Retain previous pcode function names to eliminate
    needless #defines (Rodrigo)
v5: Add new patch 4 and remove last two patches in the v4 series which will
    be submitted later. Other mostly minor fixes from code review.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>

Ashutosh Dixit (5):
  drm/i915: Introduce has_media_ratio_mode
  drm/i915/gt: Add media freq factor to per-gt sysfs
  drm/i915/pcode: Extend pcode functions for multiple gt's
  drm/i915/pcode: Init pcode on different gt's
  drm/i915/gt: Fix memory leaks in per-gt sysfs

Dale B Stimson (2):
  drm/i915/pcode: Add a couple of pcode helpers
  drm/i915/gt: Add media RP0/RPn to per-gt sysfs

 drivers/gpu/drm/i915/display/hsw_ips.c        |   4 +-
 drivers/gpu/drm/i915/display/intel_bw.c       |   6 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c    |  16 +-
 .../drm/i915/display/intel_display_power.c    |   2 +-
 .../i915/display/intel_display_power_well.c   |   4 +-
 drivers/gpu/drm/i915/display/intel_hdcp.c     |   2 +-
 drivers/gpu/drm/i915/gt/intel_gt.c            |   1 +
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |   4 +-
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       |   1 +
 drivers/gpu/drm/i915/gt/intel_gt_sysfs.c      |  29 ++-
 drivers/gpu/drm/i915/gt/intel_gt_sysfs.h      |   6 +-
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   | 177 ++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_types.h      |   3 +
 drivers/gpu/drm/i915/gt/intel_llc.c           |   3 +-
 drivers/gpu/drm/i915/gt/intel_rc6.c           |   4 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           |   4 +-
 drivers/gpu/drm/i915/gt/selftest_llc.c        |   2 +-
 drivers/gpu/drm/i915/gt/selftest_rps.c        |   2 +-
 .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h |   6 +
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   |  20 ++
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |   1 +
 .../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h |   3 +
 drivers/gpu/drm/i915/i915_driver.c            |  20 +-
 drivers/gpu/drm/i915/i915_drv.h               |   2 +
 drivers/gpu/drm/i915/i915_pci.c               |   2 +
 drivers/gpu/drm/i915/i915_reg.h               |  11 ++
 drivers/gpu/drm/i915/i915_sysfs.c             |   2 +
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 drivers/gpu/drm/i915/intel_dram.c             |   2 +-
 drivers/gpu/drm/i915/intel_pcode.c            |  93 +++++----
 drivers/gpu/drm/i915/intel_pcode.h            |  20 +-
 drivers/gpu/drm/i915/intel_pm.c               |  10 +-
 32 files changed, 363 insertions(+), 100 deletions(-)

-- 
2.34.1

[Intel-gfx] [PATCH 1/7] drm/i915: Introduce has_media_ratio_mode

[Ashutosh Dixit]

Media ratio mode (the ability for media IP to work at a different frequency
from the GT) is available for a subset of dGfx platforms supporting
GuC/SLPC. Introduce 'has_media_ratio_mode' flag in intel_device_info to
identify these platforms and set it for XEHPSDV and DG2/ATS-M.

Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 2 ++
 drivers/gpu/drm/i915/i915_pci.c          | 2 ++
 drivers/gpu/drm/i915/intel_device_info.h | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 10f273800645..3897dcb5d68d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1230,6 +1230,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define CCS_MASK(gt) \
 	ENGINE_INSTANCES_MASK(gt, CCS0, I915_MAX_CCS)
 
+#define HAS_MEDIA_RATIO_MODE(dev_priv) (INTEL_INFO(dev_priv)->has_media_ratio_mode)
+
 /*
  * The Gen7 cmdparser copies the scanned buffer to the ggtt for execution
  * All later gens can run the final buffer from the ppgtt
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index d8d893bafa51..6e309595b1e7 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1006,6 +1006,7 @@ static const struct intel_device_info xehpsdv_info = {
 	.display = { },
 	.has_64k_pages = 1,
 	.needs_compact_pt = 1,
+	.has_media_ratio_mode = 1,
 	.platform_engine_mask =
 		BIT(RCS0) | BIT(BCS0) |
 		BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) |
@@ -1027,6 +1028,7 @@ static const struct intel_device_info xehpsdv_info = {
 	.has_guc_deprivilege = 1, \
 	.has_heci_pxp = 1, \
 	.needs_compact_pt = 1, \
+	.has_media_ratio_mode = 1, \
 	.platform_engine_mask = \
 		BIT(RCS0) | BIT(BCS0) | \
 		BIT(VECS0) | BIT(VECS1) | \
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 60fc35ae81df..ebfaa6bcd508 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -152,6 +152,7 @@ enum intel_ppgtt_type {
 	func(has_l3_dpf); \
 	func(has_llc); \
 	func(has_logical_ring_contexts); \
+	func(has_media_ratio_mode); \
 	func(has_mslices); \
 	func(has_pooled_eu); \
 	func(has_pxp); \
-- 
2.34.1

[Intel-gfx] [PATCH 2/7] drm/i915/gt: Add media freq factor to per-gt sysfs

[Ashutosh Dixit]

Expose new sysfs to program and retrieve media freq factor. Factor values
of 0 (dynamic), 0.5 and 1.0 are supported via a u8.8 fixed point
representation (corresponding to integer values of 0, 128 and 256
respectively).

Media freq factor is converted to media_ratio_mode for GuC. It is
programmed into GuC using H2G SLPC interface. It is retrieved from GuC
through a register read. A cached media_ratio_mode is maintained to
preserve set values across GuC resets.

This patch adds the following sysfs files to gt/gtN sysfs:
* media_freq_factor
* media_freq_factor.scale

v2: Minor wording change in drm_warn (Tvrtko)

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       |   1 +
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c   | 130 ++++++++++++++++++
 .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h |   6 +
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   |  20 +++
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |   1 +
 .../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h |   3 +
 6 files changed, 161 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 98ede9c93f00..5dd433955861 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -740,6 +740,7 @@
 #define   GEN6_AGGRESSIVE_TURBO			(0 << 15)
 #define   GEN9_SW_REQ_UNSLICE_RATIO_SHIFT	23
 #define   GEN9_IGNORE_SLICE_RATIO		(0 << 0)
+#define   GEN12_MEDIA_FREQ_RATIO		REG_BIT(13)
 
 #define GEN6_RC_VIDEO_FREQ			_MMIO(0xa00c)
 #define   GEN6_RC_CTL_RC6pp_ENABLE		(1 << 16)
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 e92990d514b2..e3f6a889aa2e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -557,6 +557,128 @@ static const struct attribute *freq_attrs[] = {
 	NULL
 };
 
+/*
+ * Scaling for multipliers (aka frequency factors).
+ * The format of the value in the register is u8.8.
+ *
+ * The presentation to userspace is inspired by the perf event framework.
+ * See:
+ *   Documentation/ABI/testing/sysfs-bus-event_source-devices-events
+ * for description of:
+ *   /sys/bus/event_source/devices/<pmu>/events/<event>.scale
+ *
+ * Summary: Expose two sysfs files for each multiplier.
+ *
+ * 1. File <attr> contains a raw hardware value.
+ * 2. File <attr>.scale contains the multiplicative scale factor to be
+ *    used by userspace to compute the actual value.
+ *
+ * So userspace knows that to get the frequency_factor it multiplies the
+ * provided value by the specified scale factor and vice-versa.
+ *
+ * That way there is no precision loss in the kernel interface and API
+ * is future proof should one day the hardware register change to u16.u16,
+ * on some platform. (Or any other fixed point representation.)
+ *
+ * Example:
+ * File <attr> contains the value 2.5, represented as u8.8 0x0280, which
+ * is comprised of:
+ * - an integer part of 2
+ * - a fractional part of 0x80 (representing 0x80 / 2^8 == 0x80 / 256).
+ * File <attr>.scale contains a string representation of floating point
+ * value 0.00390625 (which is (1 / 256)).
+ * Userspace computes the actual value:
+ *   0x0280 * 0.00390625 -> 2.5
+ * or converts an actual value to the value to be written into <attr>:
+ *   2.5 / 0.00390625 -> 0x0280
+ */
+
+#define U8_8_VAL_MASK           0xffff
+#define U8_8_SCALE_TO_VALUE     "0.00390625"
+
+static ssize_t freq_factor_scale_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buff)
+{
+	return sysfs_emit(buff, "%s\n", U8_8_SCALE_TO_VALUE);
+}
+
+static u32 media_ratio_mode_to_factor(u32 mode)
+{
+	/* 0 -> 0, 1 -> 256, 2 -> 128 */
+	return !mode ? mode : 256 / mode;
+}
+
+static ssize_t media_freq_factor_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
+	intel_wakeref_t wakeref;
+	u32 mode;
+
+	/*
+	 * Retrieve media_ratio_mode from GEN6_RPNSWREQ bit 13 set by
+	 * GuC. GEN6_RPNSWREQ:13 value 0 represents 1:2 and 1 represents 1:1
+	 */
+	if (IS_XEHPSDV(gt->i915) &&
+	    slpc->media_ratio_mode == SLPC_MEDIA_RATIO_MODE_DYNAMIC_CONTROL) {
+		/*
+		 * For XEHPSDV dynamic mode GEN6_RPNSWREQ:13 does not contain
+		 * the media_ratio_mode, just return the cached media ratio
+		 */
+		mode = slpc->media_ratio_mode;
+	} else {
+		with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+			mode = intel_uncore_read(gt->uncore, GEN6_RPNSWREQ);
+		mode = REG_FIELD_GET(GEN12_MEDIA_FREQ_RATIO, mode) ?
+			SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_ONE :
+			SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO;
+	}
+
+	return sysfs_emit(buff, "%u\n", media_ratio_mode_to_factor(mode));
+}
+
+static ssize_t media_freq_factor_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buff, size_t count)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
+	u32 factor, mode;
+	int err;
+
+	err = kstrtou32(buff, 0, &factor);
+	if (err)
+		return err;
+
+	for (mode = SLPC_MEDIA_RATIO_MODE_DYNAMIC_CONTROL;
+	     mode <= SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO; mode++)
+		if (factor == media_ratio_mode_to_factor(mode))
+			break;
+
+	if (mode > SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO)
+		return -EINVAL;
+
+	err = intel_guc_slpc_set_media_ratio_mode(slpc, mode);
+	if (!err) {
+		slpc->media_ratio_mode = mode;
+		DRM_DEBUG("Set slpc->media_ratio_mode to %d", mode);
+	}
+	return err ?: count;
+}
+
+static DEVICE_ATTR_RW(media_freq_factor);
+static struct device_attribute dev_attr_media_freq_factor_scale =
+	__ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
+
+static const struct attribute *media_perf_power_attrs[] = {
+	&dev_attr_media_freq_factor.attr,
+	&dev_attr_media_freq_factor_scale.attr,
+	NULL
+};
+
 static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
 				const struct attribute * const *attrs)
 {
@@ -598,4 +720,12 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
 		drm_warn(&gt->i915->drm,
 			 "failed to create gt%u throttle sysfs files (%pe)",
 			 gt->info.id, ERR_PTR(ret));
+
+	if (HAS_MEDIA_RATIO_MODE(gt->i915) && intel_uc_uses_guc_slpc(&gt->uc)) {
+		ret = sysfs_create_files(kobj, media_perf_power_attrs);
+		if (ret)
+			drm_warn(&gt->i915->drm,
+				 "failed to create gt%u media_perf_power_attrs sysfs (%pe)\n",
+				 gt->info.id, ERR_PTR(ret));
+	}
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
index 62cb4254a77a..4c840a2639dc 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
@@ -122,6 +122,12 @@ enum slpc_param_id {
 	SLPC_MAX_PARAM = 32,
 };
 
+enum slpc_media_ratio_mode {
+	SLPC_MEDIA_RATIO_MODE_DYNAMIC_CONTROL = 0,
+	SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_ONE = 1,
+	SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO = 2,
+};
+
 enum slpc_event_id {
 	SLPC_EVENT_RESET = 0,
 	SLPC_EVENT_SHUTDOWN = 1,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 1db833da42df..2df31af70d63 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -260,6 +260,7 @@ int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
 	slpc->boost_freq = 0;
 	atomic_set(&slpc->num_waiters, 0);
 	slpc->num_boosts = 0;
+	slpc->media_ratio_mode = SLPC_MEDIA_RATIO_MODE_DYNAMIC_CONTROL;
 
 	mutex_init(&slpc->lock);
 	INIT_WORK(&slpc->boost_work, slpc_boost_work);
@@ -506,6 +507,22 @@ int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val)
 	return ret;
 }
 
+int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val)
+{
+	struct drm_i915_private *i915 = slpc_to_i915(slpc);
+	intel_wakeref_t wakeref;
+	int ret = 0;
+
+	if (!HAS_MEDIA_RATIO_MODE(i915))
+		return -ENODEV;
+
+	with_intel_runtime_pm(&i915->runtime_pm, wakeref)
+		ret = slpc_set_param(slpc,
+				     SLPC_PARAM_MEDIA_FF_RATIO_MODE,
+				     val);
+	return ret;
+}
+
 void intel_guc_pm_intrmsk_enable(struct intel_gt *gt)
 {
 	u32 pm_intrmsk_mbz = 0;
@@ -654,6 +671,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
 		return ret;
 	}
 
+	/* Set cached media freq ratio mode */
+	intel_guc_slpc_set_media_ratio_mode(slpc, slpc->media_ratio_mode);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
index 0caa8fee3c04..82a98f78f96c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
@@ -38,6 +38,7 @@ int intel_guc_slpc_set_boost_freq(struct intel_guc_slpc *slpc, u32 val);
 int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val);
 int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val);
 int intel_guc_slpc_print_info(struct intel_guc_slpc *slpc, struct drm_printer *p);
+int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val);
 void intel_guc_pm_intrmsk_enable(struct intel_gt *gt);
 void intel_guc_slpc_boost(struct intel_guc_slpc *slpc);
 void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
index bf5b9a563c09..73d208123528 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
@@ -29,6 +29,9 @@ struct intel_guc_slpc {
 	u32 min_freq_softlimit;
 	u32 max_freq_softlimit;
 
+	/* cached media ratio mode */
+	u32 media_ratio_mode;
+
 	/* Protects set/reset of boost freq
 	 * and value of num_waiters
 	 */
-- 
2.34.1

[Intel-gfx] [PATCH 3/7] drm/i915/pcode: Extend pcode functions for multiple gt's

[Ashutosh Dixit]

Each gt contains an independent instance of pcode. Extend pcode functions
to interface with pcode on different gt's. To avoid creating dependency of
display functionality on intel_gt, pcode function interfaces are exposed in
terms of uncore rather than intel_gt. Callers have been converted to pass
in the appropritate (i915 or intel_gt) uncore to the pcode functions.

v2: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
v3: Retain previous function names to eliminate needless #defines (Rodrigo)
v4: Move out i915_pcode_init() to a separate patch (Tvrtko)
    Remove duplicated drm_err/drm_dbg from intel_pcode_init() (Tvrtko)

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/display/hsw_ips.c        |  4 +-
 drivers/gpu/drm/i915/display/intel_bw.c       |  6 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 16 ++---
 .../drm/i915/display/intel_display_power.c    |  2 +-
 .../i915/display/intel_display_power_well.c   |  4 +-
 drivers/gpu/drm/i915/display/intel_hdcp.c     |  2 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |  4 +-
 drivers/gpu/drm/i915/gt/intel_llc.c           |  3 +-
 drivers/gpu/drm/i915/gt/intel_rc6.c           |  4 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           |  4 +-
 drivers/gpu/drm/i915/gt/selftest_llc.c        |  2 +-
 drivers/gpu/drm/i915/gt/selftest_rps.c        |  2 +-
 drivers/gpu/drm/i915/i915_driver.c            |  4 +-
 drivers/gpu/drm/i915/intel_dram.c             |  2 +-
 drivers/gpu/drm/i915/intel_pcode.c            | 69 ++++++++-----------
 drivers/gpu/drm/i915/intel_pcode.h            | 14 ++--
 drivers/gpu/drm/i915/intel_pm.c               | 10 +--
 17 files changed, 70 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/hsw_ips.c b/drivers/gpu/drm/i915/display/hsw_ips.c
index 38014e0cc9ad..861dcd2eb890 100644
--- a/drivers/gpu/drm/i915/display/hsw_ips.c
+++ b/drivers/gpu/drm/i915/display/hsw_ips.c
@@ -28,7 +28,7 @@ static void hsw_ips_enable(const struct intel_crtc_state *crtc_state)
 
 	if (IS_BROADWELL(i915)) {
 		drm_WARN_ON(&i915->drm,
-			    snb_pcode_write(i915, DISPLAY_IPS_CONTROL,
+			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL,
 					    IPS_ENABLE | IPS_PCODE_CONTROL));
 		/*
 		 * Quoting Art Runyan: "its not safe to expect any particular
@@ -62,7 +62,7 @@ bool hsw_ips_disable(const struct intel_crtc_state *crtc_state)
 
 	if (IS_BROADWELL(i915)) {
 		drm_WARN_ON(&i915->drm,
-			    snb_pcode_write(i915, DISPLAY_IPS_CONTROL, 0));
+			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL, 0));
 		/*
 		 * Wait for PCODE to finish disabling IPS. The BSpec specified
 		 * 42ms timeout value leads to occasional timeouts so use 100ms
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 37bd7b17f3d0..79269d2c476b 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -78,7 +78,7 @@ static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
 	u16 dclk;
 	int ret;
 
-	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
 			     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
 			     &val, &val2);
 	if (ret)
@@ -104,7 +104,7 @@ static int adls_pcode_read_psf_gv_point_info(struct drm_i915_private *dev_priv,
 	int ret;
 	int i;
 
-	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
 			     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO, &val, NULL);
 	if (ret)
 		return ret;
@@ -123,7 +123,7 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
 	int ret;
 
 	/* bspec says to keep retrying for at least 1 ms */
-	ret = skl_pcode_request(dev_priv, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
+	ret = skl_pcode_request(&dev_priv->uncore, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
 				points_mask,
 				ICL_PCODE_REP_QGV_MASK | ADLS_PCODE_REP_PSF_MASK,
 				ICL_PCODE_REP_QGV_SAFE | ADLS_PCODE_REP_PSF_SAFE,
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index b2017d8161b4..6e80162632dd 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -800,7 +800,7 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
 		     "trying to change cdclk frequency with cdclk not enabled\n"))
 		return;
 
-	ret = snb_pcode_write(dev_priv, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
+	ret = snb_pcode_write(&dev_priv->uncore, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
 	if (ret) {
 		drm_err(&dev_priv->drm,
 			"failed to inform pcode about cdclk change\n");
@@ -828,7 +828,7 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
 			 LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
 		drm_err(&dev_priv->drm, "Switching back to LCPLL failed\n");
 
-	snb_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
+	snb_pcode_write(&dev_priv->uncore, HSW_PCODE_DE_WRITE_FREQ_REQ,
 			cdclk_config->voltage_level);
 
 	intel_de_write(dev_priv, CDCLK_FREQ,
@@ -1086,7 +1086,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
 	drm_WARN_ON_ONCE(&dev_priv->drm,
 			 IS_SKYLAKE(dev_priv) && vco == 8640000);
 
-	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+	ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
 				SKL_CDCLK_PREPARE_FOR_CHANGE,
 				SKL_CDCLK_READY_FOR_CHANGE,
 				SKL_CDCLK_READY_FOR_CHANGE, 3);
@@ -1132,7 +1132,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
 	intel_de_posting_read(dev_priv, CDCLK_CTL);
 
 	/* inform PCU of the change */
-	snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+	snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
 			cdclk_config->voltage_level);
 
 	intel_update_cdclk(dev_priv);
@@ -1702,7 +1702,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 
 	/* Inform power controller of upcoming frequency change. */
 	if (DISPLAY_VER(dev_priv) >= 11)
-		ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+		ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
 					SKL_CDCLK_PREPARE_FOR_CHANGE,
 					SKL_CDCLK_READY_FOR_CHANGE,
 					SKL_CDCLK_READY_FOR_CHANGE, 3);
@@ -1711,7 +1711,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		 * BSpec requires us to wait up to 150usec, but that leads to
 		 * timeouts; the 2ms used here is based on experiment.
 		 */
-		ret = snb_pcode_write_timeout(dev_priv,
+		ret = snb_pcode_write_timeout(&dev_priv->uncore,
 					      HSW_PCODE_DE_WRITE_FREQ_REQ,
 					      0x80000000, 150, 2);
 	if (ret) {
@@ -1774,7 +1774,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe));
 
 	if (DISPLAY_VER(dev_priv) >= 11) {
-		ret = snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
 				      cdclk_config->voltage_level);
 	} else {
 		/*
@@ -1783,7 +1783,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		 * FIXME: Waiting for the request completion could be delayed
 		 * until the next PCODE request based on BSpec.
 		 */
-		ret = snb_pcode_write_timeout(dev_priv,
+		ret = snb_pcode_write_timeout(&dev_priv->uncore,
 					      HSW_PCODE_DE_WRITE_FREQ_REQ,
 					      cdclk_config->voltage_level,
 					      150, 2);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 1d9bd5808849..74249da35281 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1194,7 +1194,7 @@ static u32 hsw_read_dcomp(struct drm_i915_private *dev_priv)
 static void hsw_write_dcomp(struct drm_i915_private *dev_priv, u32 val)
 {
 	if (IS_HASWELL(dev_priv)) {
-		if (snb_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, val))
+		if (snb_pcode_write(&dev_priv->uncore, GEN6_PCODE_WRITE_D_COMP, val))
 			drm_dbg_kms(&dev_priv->drm,
 				    "Failed to write to D_COMP\n");
 	} else {
diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
index 5be18eb94042..91cfd5890f46 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -474,7 +474,7 @@ static void icl_tc_cold_exit(struct drm_i915_private *i915)
 	int ret, tries = 0;
 
 	while (1) {
-		ret = snb_pcode_write_timeout(i915, ICL_PCODE_EXIT_TCCOLD, 0,
+		ret = snb_pcode_write_timeout(&i915->uncore, ICL_PCODE_EXIT_TCCOLD, 0,
 					      250, 1);
 		if (ret != -EAGAIN || ++tries == 3)
 			break;
@@ -1739,7 +1739,7 @@ tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
 		 * Spec states that we should timeout the request after 200us
 		 * but the function below will timeout after 500us
 		 */
-		ret = snb_pcode_read(i915, TGL_PCODE_TCCOLD, &low_val, &high_val);
+		ret = snb_pcode_read(&i915->uncore, TGL_PCODE_TCCOLD, &low_val, &high_val);
 		if (ret == 0) {
 			if (block &&
 			    (low_val & TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 44ac0cee8b77..8ea66a2e1b09 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -298,7 +298,7 @@ static int intel_hdcp_load_keys(struct drm_i915_private *dev_priv)
 	 * Mailbox interface.
 	 */
 	if (DISPLAY_VER(dev_priv) == 9 && !IS_BROXTON(dev_priv)) {
-		ret = snb_pcode_write(dev_priv, SKL_PCODE_LOAD_HDCP_KEYS, 1);
+		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_LOAD_HDCP_KEYS, 1);
 		if (ret) {
 			drm_err(&dev_priv->drm,
 				"Failed to initiate HDCP key load (%d)\n",
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 0c6b9eb724ae..90a440865037 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -138,7 +138,7 @@ static int gen6_drpc(struct seq_file *m)
 	}
 
 	if (GRAPHICS_VER(i915) <= 7)
-		snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
+		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
 
 	seq_printf(m, "RC1e Enabled: %s\n",
 		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC1e_ENABLE));
@@ -545,7 +545,7 @@ static int llc_show(struct seq_file *m, void *data)
 	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
 	for (gpu_freq = min_gpu_freq; gpu_freq <= max_gpu_freq; gpu_freq++) {
 		ia_freq = gpu_freq;
-		snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
+		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
 			       &ia_freq, NULL);
 		seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
 			   intel_gpu_freq(rps,
diff --git a/drivers/gpu/drm/i915/gt/intel_llc.c b/drivers/gpu/drm/i915/gt/intel_llc.c
index 40e2e28ee6c7..14fe65812e42 100644
--- a/drivers/gpu/drm/i915/gt/intel_llc.c
+++ b/drivers/gpu/drm/i915/gt/intel_llc.c
@@ -124,7 +124,6 @@ static void calc_ia_freq(struct intel_llc *llc,
 
 static void gen6_update_ring_freq(struct intel_llc *llc)
 {
-	struct drm_i915_private *i915 = llc_to_gt(llc)->i915;
 	struct ia_constants consts;
 	unsigned int gpu_freq;
 
@@ -142,7 +141,7 @@ static void gen6_update_ring_freq(struct intel_llc *llc)
 		unsigned int ia_freq, ring_freq;
 
 		calc_ia_freq(llc, gpu_freq, &consts, &ia_freq, &ring_freq);
-		snb_pcode_write(i915, GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
+		snb_pcode_write(llc_to_gt(llc)->uncore, GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
 				ia_freq << GEN6_PCODE_FREQ_IA_RATIO_SHIFT |
 				ring_freq << GEN6_PCODE_FREQ_RING_RATIO_SHIFT |
 				gpu_freq);
diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
index b4770690e794..f8d0523f4c18 100644
--- a/drivers/gpu/drm/i915/gt/intel_rc6.c
+++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
@@ -272,7 +272,7 @@ static void gen6_rc6_enable(struct intel_rc6 *rc6)
 	    GEN6_RC_CTL_HW_ENABLE;
 
 	rc6vids = 0;
-	ret = snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
+	ret = snb_pcode_read(rc6_to_gt(rc6)->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
 	if (GRAPHICS_VER(i915) == 6 && ret) {
 		drm_dbg(&i915->drm, "Couldn't check for BIOS workaround\n");
 	} else if (GRAPHICS_VER(i915) == 6 &&
@@ -282,7 +282,7 @@ static void gen6_rc6_enable(struct intel_rc6 *rc6)
 			GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
 		rc6vids &= 0xffff00;
 		rc6vids |= GEN6_ENCODE_RC6_VID(450);
-		ret = snb_pcode_write(i915, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
+		ret = snb_pcode_write(rc6_to_gt(rc6)->uncore, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
 		if (ret)
 			drm_err(&i915->drm,
 				"Couldn't fix incorrect rc6 voltage\n");
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 3bd8415a0f1b..a62d323ff056 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1144,7 +1144,7 @@ static void gen6_rps_init(struct intel_rps *rps)
 
 		if (IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11)
 			mult = GEN9_FREQ_SCALER;
-		if (snb_pcode_read(i915, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
+		if (snb_pcode_read(rps_to_gt(rps)->uncore, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
 				   &ddcc_status, NULL) == 0)
 			rps->efficient_freq =
 				clamp_t(u32,
@@ -1984,7 +1984,7 @@ void intel_rps_init(struct intel_rps *rps)
 	if (GRAPHICS_VER(i915) == 6 || IS_IVYBRIDGE(i915) || IS_HASWELL(i915)) {
 		u32 params = 0;
 
-		snb_pcode_read(i915, GEN6_READ_OC_PARAMS, &params, NULL);
+		snb_pcode_read(rps_to_gt(rps)->uncore, GEN6_READ_OC_PARAMS, &params, NULL);
 		if (params & BIT(31)) { /* OC supported */
 			drm_dbg(&i915->drm,
 				"Overclocking supported, max: %dMHz, overclock: %dMHz\n",
diff --git a/drivers/gpu/drm/i915/gt/selftest_llc.c b/drivers/gpu/drm/i915/gt/selftest_llc.c
index 2cd184ab32b1..cfd736d88939 100644
--- a/drivers/gpu/drm/i915/gt/selftest_llc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_llc.c
@@ -31,7 +31,7 @@ static int gen6_verify_ring_freq(struct intel_llc *llc)
 		calc_ia_freq(llc, gpu_freq, &consts, &ia_freq, &ring_freq);
 
 		val = gpu_freq;
-		if (snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
+		if (snb_pcode_read(llc_to_gt(llc)->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
 				   &val, NULL)) {
 			pr_err("Failed to read freq table[%d], range [%d, %d]\n",
 			       gpu_freq, consts.min_gpu_freq, consts.max_gpu_freq);
diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
index 6a69ac0184ad..cfb4708dd62e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
@@ -521,7 +521,7 @@ static void show_pcu_config(struct intel_rps *rps)
 	for (gpu_freq = min_gpu_freq; gpu_freq <= max_gpu_freq; gpu_freq++) {
 		int ia_freq = gpu_freq;
 
-		snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
+		snb_pcode_read(rps_to_gt(rps)->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
 			       &ia_freq, NULL);
 
 		pr_info("%5d  %5d  %5d\n",
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 90b0ce5051af..0e9763868d68 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -629,7 +629,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
 
 	intel_opregion_setup(dev_priv);
 
-	ret = intel_pcode_init(dev_priv);
+	ret = intel_pcode_init(&dev_priv->uncore);
 	if (ret)
 		goto err_msi;
 
@@ -1251,7 +1251,7 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
-	ret = intel_pcode_init(dev_priv);
+	ret = intel_pcode_init(&dev_priv->uncore);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index 2b9e7833da96..437447119770 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -393,7 +393,7 @@ static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
 	u32 val = 0;
 	int ret;
 
-	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
 			     ICL_PCODE_MEM_SS_READ_GLOBAL_INFO, &val, NULL);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c
index ac727546868e..2be700932322 100644
--- a/drivers/gpu/drm/i915/intel_pcode.c
+++ b/drivers/gpu/drm/i915/intel_pcode.c
@@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox)
 	}
 }
 
-static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
+static int __snb_pcode_rw(struct intel_uncore *uncore, u32 mbox,
 			  u32 *val, u32 *val1,
 			  int fast_timeout_us, int slow_timeout_ms,
 			  bool is_read)
 {
-	struct intel_uncore *uncore = &i915->uncore;
-
-	lockdep_assert_held(&i915->sb_lock);
+	lockdep_assert_held(&uncore->i915->sb_lock);
 
 	/*
 	 * GEN6_PCODE_* are outside of the forcewake domain, we can use
@@ -88,22 +86,22 @@ static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
 	if (is_read && val1)
 		*val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
 
-	if (GRAPHICS_VER(i915) > 6)
+	if (GRAPHICS_VER(uncore->i915) > 6)
 		return gen7_check_mailbox_status(mbox);
 	else
 		return gen6_check_mailbox_status(mbox);
 }
 
-int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1)
+int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1)
 {
 	int err;
 
-	mutex_lock(&i915->sb_lock);
-	err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
-	mutex_unlock(&i915->sb_lock);
+	mutex_lock(&uncore->i915->sb_lock);
+	err = __snb_pcode_rw(uncore, mbox, val, val1, 500, 20, true);
+	mutex_unlock(&uncore->i915->sb_lock);
 
 	if (err) {
-		drm_dbg(&i915->drm,
+		drm_dbg(&uncore->i915->drm,
 			"warning: pcode (read from mbox %x) mailbox access failed for %ps: %d\n",
 			mbox, __builtin_return_address(0), err);
 	}
@@ -111,18 +109,18 @@ int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1)
 	return err;
 }
 
-int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
+int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val,
 			    int fast_timeout_us, int slow_timeout_ms)
 {
 	int err;
 
-	mutex_lock(&i915->sb_lock);
-	err = __snb_pcode_rw(i915, mbox, &val, NULL,
+	mutex_lock(&uncore->i915->sb_lock);
+	err = __snb_pcode_rw(uncore, mbox, &val, NULL,
 			     fast_timeout_us, slow_timeout_ms, false);
-	mutex_unlock(&i915->sb_lock);
+	mutex_unlock(&uncore->i915->sb_lock);
 
 	if (err) {
-		drm_dbg(&i915->drm,
+		drm_dbg(&uncore->i915->drm,
 			"warning: pcode (write of 0x%08x to mbox %x) mailbox access failed for %ps: %d\n",
 			val, mbox, __builtin_return_address(0), err);
 	}
@@ -130,18 +128,18 @@ int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
 	return err;
 }
 
-static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
+static bool skl_pcode_try_request(struct intel_uncore *uncore, u32 mbox,
 				  u32 request, u32 reply_mask, u32 reply,
 				  u32 *status)
 {
-	*status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
+	*status = __snb_pcode_rw(uncore, mbox, &request, NULL, 500, 0, true);
 
 	return (*status == 0) && ((request & reply_mask) == reply);
 }
 
 /**
  * skl_pcode_request - send PCODE request until acknowledgment
- * @i915: device private
+ * @uncore: uncore
  * @mbox: PCODE mailbox ID the request is targeted for
  * @request: request ID
  * @reply_mask: mask used to check for request acknowledgment
@@ -158,16 +156,16 @@ static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
  * Returns 0 on success, %-ETIMEDOUT in case of a timeout, <0 in case of some
  * other error as reported by PCODE.
  */
-int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
+int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
 		      u32 reply_mask, u32 reply, int timeout_base_ms)
 {
 	u32 status;
 	int ret;
 
-	mutex_lock(&i915->sb_lock);
+	mutex_lock(&uncore->i915->sb_lock);
 
 #define COND \
-	skl_pcode_try_request(i915, mbox, request, reply_mask, reply, &status)
+	skl_pcode_try_request(uncore, mbox, request, reply_mask, reply, &status)
 
 	/*
 	 * Prime the PCODE by doing a request first. Normally it guarantees
@@ -193,35 +191,26 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
 	 * requests, and for any quirks of the PCODE firmware that delays
 	 * the request completion.
 	 */
-	drm_dbg_kms(&i915->drm,
+	drm_dbg_kms(&uncore->i915->drm,
 		    "PCODE timeout, retrying with preemption disabled\n");
-	drm_WARN_ON_ONCE(&i915->drm, timeout_base_ms > 3);
+	drm_WARN_ON_ONCE(&uncore->i915->drm, timeout_base_ms > 3);
 	preempt_disable();
 	ret = wait_for_atomic(COND, 50);
 	preempt_enable();
 
 out:
-	mutex_unlock(&i915->sb_lock);
+	mutex_unlock(&uncore->i915->sb_lock);
 	return status ? status : ret;
 #undef COND
 }
 
-int intel_pcode_init(struct drm_i915_private *i915)
+int intel_pcode_init(struct intel_uncore *uncore)
 {
-	int ret = 0;
-
-	if (!IS_DGFX(i915))
-		return ret;
-
-	ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
-				DG1_UNCORE_GET_INIT_STATUS,
-				DG1_UNCORE_INIT_STATUS_COMPLETE,
-				DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
-
-	drm_dbg(&i915->drm, "PCODE init status %d\n", ret);
-
-	if (ret)
-		drm_err(&i915->drm, "Pcode did not report uncore initialization completion!\n");
+	if (!IS_DGFX(uncore->i915))
+		return 0;
 
-	return ret;
+	return skl_pcode_request(uncore, DG1_PCODE_STATUS,
+				 DG1_UNCORE_GET_INIT_STATUS,
+				 DG1_UNCORE_INIT_STATUS_COMPLETE,
+				 DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
 }
diff --git a/drivers/gpu/drm/i915/intel_pcode.h b/drivers/gpu/drm/i915/intel_pcode.h
index 0962a17fac48..8f6241b114a5 100644
--- a/drivers/gpu/drm/i915/intel_pcode.h
+++ b/drivers/gpu/drm/i915/intel_pcode.h
@@ -8,17 +8,17 @@
 
 #include <linux/types.h>
 
-struct drm_i915_private;
+struct intel_uncore;
 
-int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1);
-int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
+int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1);
+int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val,
 			    int fast_timeout_us, int slow_timeout_ms);
-#define snb_pcode_write(i915, mbox, val)			\
-	snb_pcode_write_timeout(i915, mbox, val, 500, 0)
+#define snb_pcode_write(uncore, mbox, val) \
+	snb_pcode_write_timeout(uncore, mbox, val, 500, 0)
 
-int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
+int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
 		      u32 reply_mask, u32 reply, int timeout_base_ms);
 
-int intel_pcode_init(struct drm_i915_private *i915);
+int intel_pcode_init(struct intel_uncore *uncore);
 
 #endif /* _INTEL_PCODE_H */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ee0047fdc95d..aacb21cbc62e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2874,7 +2874,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 
 		/* read the first set of memory latencies[0:3] */
 		val = 0; /* data0 to be programmed to 0 for first set */
-		ret = snb_pcode_read(dev_priv, GEN9_PCODE_READ_MEM_LATENCY,
+		ret = snb_pcode_read(&dev_priv->uncore, GEN9_PCODE_READ_MEM_LATENCY,
 				     &val, NULL);
 
 		if (ret) {
@@ -2893,7 +2893,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 
 		/* read the second set of memory latencies[4:7] */
 		val = 1; /* data0 to be programmed to 1 for second set */
-		ret = snb_pcode_read(dev_priv, GEN9_PCODE_READ_MEM_LATENCY,
+		ret = snb_pcode_read(&dev_priv->uncore, GEN9_PCODE_READ_MEM_LATENCY,
 				     &val, NULL);
 		if (ret) {
 			drm_err(&dev_priv->drm,
@@ -3679,7 +3679,7 @@ intel_sagv_block_time(struct drm_i915_private *dev_priv)
 		u32 val = 0;
 		int ret;
 
-		ret = snb_pcode_read(dev_priv,
+		ret = snb_pcode_read(&dev_priv->uncore,
 				     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
 				     &val, NULL);
 		if (ret) {
@@ -3748,7 +3748,7 @@ static void skl_sagv_enable(struct drm_i915_private *dev_priv)
 		return;
 
 	drm_dbg_kms(&dev_priv->drm, "Enabling SAGV\n");
-	ret = snb_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,
+	ret = snb_pcode_write(&dev_priv->uncore, GEN9_PCODE_SAGV_CONTROL,
 			      GEN9_SAGV_ENABLE);
 
 	/* We don't need to wait for SAGV when enabling */
@@ -3781,7 +3781,7 @@ static void skl_sagv_disable(struct drm_i915_private *dev_priv)
 
 	drm_dbg_kms(&dev_priv->drm, "Disabling SAGV\n");
 	/* bspec says to keep retrying for at least 1 ms */
-	ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL,
+	ret = skl_pcode_request(&dev_priv->uncore, GEN9_PCODE_SAGV_CONTROL,
 				GEN9_SAGV_DISABLE,
 				GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
 				1);
-- 
2.34.1

[Intel-gfx] [PATCH 4/7] drm/i915/pcode: Init pcode on different gt's

[Ashutosh Dixit]

Extend pcode initialization to pcode on different gt's.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 0e9763868d68..e137bcf021ee 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -520,6 +520,22 @@ static int i915_set_dma_info(struct drm_i915_private *i915)
 	return ret;
 }
 
+static int i915_pcode_init(struct drm_i915_private *i915)
+{
+	struct intel_gt *gt;
+	int id, ret;
+
+	for_each_gt(gt, i915, id) {
+		ret = intel_pcode_init(gt->uncore);
+		if (ret) {
+			drm_err(&gt->i915->drm, "gt%d: intel_pcode_init failed %d\n", id, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * i915_driver_hw_probe - setup state requiring device access
  * @dev_priv: device private
@@ -629,7 +645,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
 
 	intel_opregion_setup(dev_priv);
 
-	ret = intel_pcode_init(&dev_priv->uncore);
+	ret = i915_pcode_init(dev_priv);
 	if (ret)
 		goto err_msi;
 
@@ -1251,7 +1267,7 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
-	ret = intel_pcode_init(&dev_priv->uncore);
+	ret = i915_pcode_init(dev_priv);
 	if (ret)
 		return ret;
 
-- 
2.34.1

[Intel-gfx] [PATCH 5/7] drm/i915/pcode: Add a couple of pcode helpers

[Ashutosh Dixit]

From: Dale B Stimson <dale.b.stimson@intel.com>

Some dGfx pcode commands take additional sub-commands and parameters. Add a
couple of helpers to help formatting these commands to improve code
readability.

v2: Fixed commit author (Rodrigo)
v3: Function rename and convert to new uncore interface for pcode functions
    Remove unnecessary #define's (Andi)
v4: Another function rename

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  3 +++
 drivers/gpu/drm/i915/intel_pcode.c | 32 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pcode.h |  6 ++++++
 3 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ab64ab4317b3..0e04345248ea 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6698,6 +6698,9 @@
 
 #define GEN6_PCODE_MAILBOX			_MMIO(0x138124)
 #define   GEN6_PCODE_READY			(1 << 31)
+#define   GEN6_PCODE_MB_PARAM2			REG_GENMASK(23, 16)
+#define   GEN6_PCODE_MB_PARAM1			REG_GENMASK(15, 8)
+#define   GEN6_PCODE_MB_COMMAND			REG_GENMASK(7, 0)
 #define   GEN6_PCODE_ERROR_MASK			0xFF
 #define     GEN6_PCODE_SUCCESS			0x0
 #define     GEN6_PCODE_ILLEGAL_CMD		0x1
diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c
index 2be700932322..a234d9b4ed14 100644
--- a/drivers/gpu/drm/i915/intel_pcode.c
+++ b/drivers/gpu/drm/i915/intel_pcode.c
@@ -214,3 +214,35 @@ int intel_pcode_init(struct intel_uncore *uncore)
 				 DG1_UNCORE_INIT_STATUS_COMPLETE,
 				 DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
 }
+
+int snb_pcode_read_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 *val)
+{
+	intel_wakeref_t wakeref;
+	u32 mbox;
+	int err;
+
+	mbox = REG_FIELD_PREP(GEN6_PCODE_MB_COMMAND, mbcmd)
+		| REG_FIELD_PREP(GEN6_PCODE_MB_PARAM1, p1)
+		| REG_FIELD_PREP(GEN6_PCODE_MB_PARAM2, p2);
+
+	with_intel_runtime_pm(uncore->rpm, wakeref)
+		err = snb_pcode_read(uncore, mbox, val, NULL);
+
+	return err;
+}
+
+int snb_pcode_write_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 val)
+{
+	intel_wakeref_t wakeref;
+	u32 mbox;
+	int err;
+
+	mbox = REG_FIELD_PREP(GEN6_PCODE_MB_COMMAND, mbcmd)
+		| REG_FIELD_PREP(GEN6_PCODE_MB_PARAM1, p1)
+		| REG_FIELD_PREP(GEN6_PCODE_MB_PARAM2, p2);
+
+	with_intel_runtime_pm(uncore->rpm, wakeref)
+		err = snb_pcode_write(uncore, mbox, val);
+
+	return err;
+}
diff --git a/drivers/gpu/drm/i915/intel_pcode.h b/drivers/gpu/drm/i915/intel_pcode.h
index 8f6241b114a5..8d2198e29422 100644
--- a/drivers/gpu/drm/i915/intel_pcode.h
+++ b/drivers/gpu/drm/i915/intel_pcode.h
@@ -21,4 +21,10 @@ int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
 
 int intel_pcode_init(struct intel_uncore *uncore);
 
+/*
+ * Helpers for dGfx PCODE mailbox command formatting
+ */
+int snb_pcode_read_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 *val);
+int snb_pcode_write_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 val);
+
 #endif /* _INTEL_PCODE_H */
-- 
2.34.1

[Intel-gfx] [PATCH 6/7] drm/i915/gt: Add media RP0/RPn to per-gt sysfs

[Ashutosh Dixit]

From: Dale B Stimson <dale.b.stimson@intel.com>

Retrieve RP0 and RPn freq for media IP from PCODE and display in per-gt
sysfs. This patch adds the following files to gt/gtN sysfs:
* media_RP0_freq_mhz
* media_RPn_freq_mhz

v2: Fixed commit author (Rodrigo)
v3: Convert to new uncore interface for pcode functions
v4: Adapt to intel_pcode.* function rename
v5: #include "intel_pcode.h" in alphabetical order (Tvrtko)

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 47 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h             |  8 ++++
 2 files changed, 55 insertions(+)

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 e3f6a889aa2e..79a2fa86947a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -14,6 +14,7 @@
 #include "intel_gt_regs.h"
 #include "intel_gt_sysfs.h"
 #include "intel_gt_sysfs_pm.h"
+#include "intel_pcode.h"
 #include "intel_rc6.h"
 #include "intel_rps.h"
 
@@ -669,13 +670,59 @@ static ssize_t media_freq_factor_store(struct device *dev,
 	return err ?: count;
 }
 
+static ssize_t media_RP0_freq_mhz_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	u32 val;
+	int err;
+
+	err = snb_pcode_read_p(gt->uncore, XEHPSDV_PCODE_FREQUENCY_CONFIG,
+			       PCODE_MBOX_FC_SC_READ_FUSED_P0,
+			       PCODE_MBOX_DOMAIN_MEDIAFF, &val);
+
+	if (err)
+		return err;
+
+	/* Fused media RP0 read from pcode is in units of 50 MHz */
+	val *= GT_FREQUENCY_MULTIPLIER;
+
+	return sysfs_emit(buff, "%u\n", val);
+}
+
+static ssize_t media_RPn_freq_mhz_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	u32 val;
+	int err;
+
+	err = snb_pcode_read_p(gt->uncore, XEHPSDV_PCODE_FREQUENCY_CONFIG,
+			       PCODE_MBOX_FC_SC_READ_FUSED_PN,
+			       PCODE_MBOX_DOMAIN_MEDIAFF, &val);
+
+	if (err)
+		return err;
+
+	/* Fused media RPn read from pcode is in units of 50 MHz */
+	val *= GT_FREQUENCY_MULTIPLIER;
+
+	return sysfs_emit(buff, "%u\n", val);
+}
+
 static DEVICE_ATTR_RW(media_freq_factor);
 static struct device_attribute dev_attr_media_freq_factor_scale =
 	__ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
+static DEVICE_ATTR_RO(media_RP0_freq_mhz);
+static DEVICE_ATTR_RO(media_RPn_freq_mhz);
 
 static const struct attribute *media_perf_power_attrs[] = {
 	&dev_attr_media_freq_factor.attr,
 	&dev_attr_media_freq_factor_scale.attr,
+	&dev_attr_media_RP0_freq_mhz.attr,
+	&dev_attr_media_RPn_freq_mhz.attr,
 	NULL
 };
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0e04345248ea..48d41467ce24 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6767,6 +6767,14 @@
 #define     DG1_UNCORE_GET_INIT_STATUS		0x0
 #define     DG1_UNCORE_INIT_STATUS_COMPLETE	0x1
 #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
+#define   XEHPSDV_PCODE_FREQUENCY_CONFIG		0x6e	/* xehpsdv, pvc */
+/* XEHPSDV_PCODE_FREQUENCY_CONFIG sub-commands (param1) */
+#define     PCODE_MBOX_FC_SC_READ_FUSED_P0	0x0
+#define     PCODE_MBOX_FC_SC_READ_FUSED_PN	0x1
+/* PCODE_MBOX_DOMAIN_* - mailbox domain IDs */
+/*   XEHPSDV_PCODE_FREQUENCY_CONFIG param2 */
+#define     PCODE_MBOX_DOMAIN_NONE		0x0
+#define     PCODE_MBOX_DOMAIN_MEDIAFF		0x3
 #define GEN6_PCODE_DATA				_MMIO(0x138128)
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
-- 
2.34.1

[Intel-gfx] [PATCH 7/7] drm/i915/gt: Fix memory leaks in per-gt sysfs

[Ashutosh Dixit]

All kmalloc'd kobjects need a kobject_put() to free memory. For example in
previous code, kobj_gt_release() never gets called. The requirement of
kobject_put() now results in a slightly different code organization.

v2: s/gtn/gt/ (Andi)

Fixes: b770bcfae9ad ("drm/i915/gt: create per-tile sysfs interface")
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Acked-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c       |  1 +
 drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 29 ++++++++++--------------
 drivers/gpu/drm/i915/gt/intel_gt_sysfs.h |  6 +----
 drivers/gpu/drm/i915/gt/intel_gt_types.h |  3 +++
 drivers/gpu/drm/i915/i915_sysfs.c        |  2 ++
 5 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 034182f85501..0a3931c011c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -790,6 +790,7 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
 {
 	intel_wakeref_t wakeref;
 
+	intel_gt_sysfs_unregister(gt);
 	intel_rps_driver_unregister(&gt->rps);
 	intel_gsc_fini(&gt->gsc);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
index 8ec8bc660c8c..9e4ebf53379b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
@@ -24,7 +24,7 @@ bool is_object_gt(struct kobject *kobj)
 
 static struct intel_gt *kobj_to_gt(struct kobject *kobj)
 {
-	return container_of(kobj, struct kobj_gt, base)->gt;
+	return container_of(kobj, struct intel_gt, sysfs_gt);
 }
 
 struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
@@ -72,9 +72,9 @@ static struct attribute *id_attrs[] = {
 };
 ATTRIBUTE_GROUPS(id);
 
+/* A kobject needs a release() method even if it does nothing */
 static void kobj_gt_release(struct kobject *kobj)
 {
-	kfree(kobj);
 }
 
 static struct kobj_type kobj_gt_type = {
@@ -85,8 +85,6 @@ static struct kobj_type kobj_gt_type = {
 
 void intel_gt_sysfs_register(struct intel_gt *gt)
 {
-	struct kobj_gt *kg;
-
 	/*
 	 * We need to make things right with the
 	 * ABI compatibility. The files were originally
@@ -98,25 +96,22 @@ void intel_gt_sysfs_register(struct intel_gt *gt)
 	if (gt_is_root(gt))
 		intel_gt_sysfs_pm_init(gt, gt_get_parent_obj(gt));
 
-	kg = kzalloc(sizeof(*kg), GFP_KERNEL);
-	if (!kg)
+	/* init and xfer ownership to sysfs tree */
+	if (kobject_init_and_add(&gt->sysfs_gt, &kobj_gt_type,
+				 gt->i915->sysfs_gt, "gt%d", gt->info.id))
 		goto exit_fail;
 
-	kobject_init(&kg->base, &kobj_gt_type);
-	kg->gt = gt;
-
-	/* xfer ownership to sysfs tree */
-	if (kobject_add(&kg->base, gt->i915->sysfs_gt, "gt%d", gt->info.id))
-		goto exit_kobj_put;
-
-	intel_gt_sysfs_pm_init(gt, &kg->base);
+	intel_gt_sysfs_pm_init(gt, &gt->sysfs_gt);
 
 	return;
 
-exit_kobj_put:
-	kobject_put(&kg->base);
-
 exit_fail:
+	kobject_put(&gt->sysfs_gt);
 	drm_warn(&gt->i915->drm,
 		 "failed to initialize gt%d sysfs root\n", gt->info.id);
 }
+
+void intel_gt_sysfs_unregister(struct intel_gt *gt)
+{
+	kobject_put(&gt->sysfs_gt);
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
index 9471b26752cf..a99aa7e8b01a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
@@ -13,11 +13,6 @@
 
 struct intel_gt;
 
-struct kobj_gt {
-	struct kobject base;
-	struct intel_gt *gt;
-};
-
 bool is_object_gt(struct kobject *kobj);
 
 struct drm_i915_private *kobj_to_i915(struct kobject *kobj);
@@ -28,6 +23,7 @@ intel_gt_create_kobj(struct intel_gt *gt,
 		     const char *name);
 
 void intel_gt_sysfs_register(struct intel_gt *gt);
+void intel_gt_sysfs_unregister(struct intel_gt *gt);
 struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
 					    const char *name);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 097e10291f2d..993f003dad1d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -225,6 +225,9 @@ struct intel_gt {
 	} mocs;
 
 	struct intel_pxp pxp;
+
+	/* gt/gtN sysfs */
+	struct kobject sysfs_gt;
 };
 
 enum intel_gt_scratch_field {
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 8521daba212a..3f06106cdcf5 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -259,4 +259,6 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
 
 	device_remove_bin_file(kdev,  &dpf_attrs_1);
 	device_remove_bin_file(kdev,  &dpf_attrs);
+
+	kobject_put(dev_priv->sysfs_gt);
 }
-- 
2.34.1

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Media freq factor and per-gt enhancements/fixes (rev5)

[Patchwork]

== Series Details ==

Series: drm/i915: Media freq factor and per-gt enhancements/fixes (rev5)
URL   : https://patchwork.freedesktop.org/series/102665/
State : warning

== Summary ==

Error: dim checkpatch failed
9401a3134905 drm/i915: Introduce has_media_ratio_mode
1e160dfda490 drm/i915/gt: Add media freq factor to per-gt sysfs
764c4b9a031c drm/i915/pcode: Extend pcode functions for multiple gt's
38ff293baf98 drm/i915/pcode: Init pcode on different gt's
19e1f61d7a89 drm/i915/pcode: Add a couple of pcode helpers
90bcfd2eca3d drm/i915/gt: Add media RP0/RPn to per-gt sysfs
-:83: CHECK:CAMELCASE: Avoid CamelCase: <media_RPn_freq_mhz>
#83: FILE: drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:719:
+static DEVICE_ATTR_RO(media_RPn_freq_mhz);

-:89: CHECK:CAMELCASE: Avoid CamelCase: <dev_attr_media_RPn_freq_mhz>
#89: FILE: drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:725:
+	&dev_attr_media_RPn_freq_mhz.attr,

total: 0 errors, 0 warnings, 2 checks, 80 lines checked
5b92a3affd8b drm/i915/gt: Fix memory leaks in per-gt sysfs

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Media freq factor and per-gt enhancements/fixes (rev5)

[Patchwork]

== Series Details ==

Series: drm/i915: Media freq factor and per-gt enhancements/fixes (rev5)
URL   : https://patchwork.freedesktop.org/series/102665/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

Re: [Intel-gfx] [PATCH v5 0/7] drm/i915: Media freq factor and per-gt enhancements/fixes

[Dixit, Ashutosh]

On Wed, 11 May 2022 19:32:13 -0700, Ashutosh Dixit wrote:
>
> Some recent Intel dGfx platforms allow media IP to work at a different
> frequency from the base GT. This patch series exposes sysfs controls for
> this functionality in the new per-gt sysfs. Some enhancements and fixes to
> previous per-gt functionality are also included to complete the new
> functionality:
> * Patches 1 and 2 implement basic sysfs controls for media freq
> * Patch 3 extends previous pcode functions for multiple gt's
> * Patch 4 inits pcode on different gt's
> * Patch 5 adds a couple of pcode helpers
> * Patch 6 uses the new pcode functions to retrieve media RP0/RPn freq
> * Patch 7 fixes memory leaks in the previous per-gt sysfs implementation
>   and some code refactoring

In this v5 I have dropped the last two patches of the v4 series, these
ones:

[PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs
[PATCH 8/8] drm/i915/gt: Expose default value for media_freq_factor in per-gt sysfs

Because these need more work based on the review comments. If this series
is merged I will submit these patches as a separate series, otherwise I
will re-add them to this series and resubmit (due to dependence between
this series and those patches).

Apart from this, I believe I have addressed all previous review comments on
the patches in this series.

Thanks for reviewing,
Ashutosh

Re: [Intel-gfx] [PATCH 3/7] drm/i915/pcode: Extend pcode functions for multiple gt's

[Tvrtko Ursulin]

On 12/05/2022 03:32, Ashutosh Dixit wrote:
> Each gt contains an independent instance of pcode. Extend pcode functions
> to interface with pcode on different gt's. To avoid creating dependency of
> display functionality on intel_gt, pcode function interfaces are exposed in
> terms of uncore rather than intel_gt. Callers have been converted to pass
> in the appropritate (i915 or intel_gt) uncore to the pcode functions.
> 
> v2: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
> v3: Retain previous function names to eliminate needless #defines (Rodrigo)
> v4: Move out i915_pcode_init() to a separate patch (Tvrtko)
>      Remove duplicated drm_err/drm_dbg from intel_pcode_init() (Tvrtko)
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/hsw_ips.c        |  4 +-
>   drivers/gpu/drm/i915/display/intel_bw.c       |  6 +-
>   drivers/gpu/drm/i915/display/intel_cdclk.c    | 16 ++---
>   .../drm/i915/display/intel_display_power.c    |  2 +-
>   .../i915/display/intel_display_power_well.c   |  4 +-
>   drivers/gpu/drm/i915/display/intel_hdcp.c     |  2 +-
>   drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |  4 +-
>   drivers/gpu/drm/i915/gt/intel_llc.c           |  3 +-
>   drivers/gpu/drm/i915/gt/intel_rc6.c           |  4 +-
>   drivers/gpu/drm/i915/gt/intel_rps.c           |  4 +-
>   drivers/gpu/drm/i915/gt/selftest_llc.c        |  2 +-
>   drivers/gpu/drm/i915/gt/selftest_rps.c        |  2 +-
>   drivers/gpu/drm/i915/i915_driver.c            |  4 +-
>   drivers/gpu/drm/i915/intel_dram.c             |  2 +-
>   drivers/gpu/drm/i915/intel_pcode.c            | 69 ++++++++-----------
>   drivers/gpu/drm/i915/intel_pcode.h            | 14 ++--
>   drivers/gpu/drm/i915/intel_pm.c               | 10 +--
>   17 files changed, 70 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/hsw_ips.c b/drivers/gpu/drm/i915/display/hsw_ips.c
> index 38014e0cc9ad..861dcd2eb890 100644
> --- a/drivers/gpu/drm/i915/display/hsw_ips.c
> +++ b/drivers/gpu/drm/i915/display/hsw_ips.c
> @@ -28,7 +28,7 @@ static void hsw_ips_enable(const struct intel_crtc_state *crtc_state)
>   
>   	if (IS_BROADWELL(i915)) {
>   		drm_WARN_ON(&i915->drm,
> -			    snb_pcode_write(i915, DISPLAY_IPS_CONTROL,
> +			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL,
>   					    IPS_ENABLE | IPS_PCODE_CONTROL));
>   		/*
>   		 * Quoting Art Runyan: "its not safe to expect any particular
> @@ -62,7 +62,7 @@ bool hsw_ips_disable(const struct intel_crtc_state *crtc_state)
>   
>   	if (IS_BROADWELL(i915)) {
>   		drm_WARN_ON(&i915->drm,
> -			    snb_pcode_write(i915, DISPLAY_IPS_CONTROL, 0));
> +			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL, 0));
>   		/*
>   		 * Wait for PCODE to finish disabling IPS. The BSpec specified
>   		 * 42ms timeout value leads to occasional timeouts so use 100ms
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 37bd7b17f3d0..79269d2c476b 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -78,7 +78,7 @@ static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>   	u16 dclk;
>   	int ret;
>   
> -	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>   			     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
>   			     &val, &val2);
>   	if (ret)
> @@ -104,7 +104,7 @@ static int adls_pcode_read_psf_gv_point_info(struct drm_i915_private *dev_priv,
>   	int ret;
>   	int i;
>   
> -	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>   			     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO, &val, NULL);
>   	if (ret)
>   		return ret;
> @@ -123,7 +123,7 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
>   	int ret;
>   
>   	/* bspec says to keep retrying for at least 1 ms */
> -	ret = skl_pcode_request(dev_priv, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
> +	ret = skl_pcode_request(&dev_priv->uncore, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
>   				points_mask,
>   				ICL_PCODE_REP_QGV_MASK | ADLS_PCODE_REP_PSF_MASK,
>   				ICL_PCODE_REP_QGV_SAFE | ADLS_PCODE_REP_PSF_SAFE,
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index b2017d8161b4..6e80162632dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -800,7 +800,7 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
>   		     "trying to change cdclk frequency with cdclk not enabled\n"))
>   		return;
>   
> -	ret = snb_pcode_write(dev_priv, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> +	ret = snb_pcode_write(&dev_priv->uncore, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
>   	if (ret) {
>   		drm_err(&dev_priv->drm,
>   			"failed to inform pcode about cdclk change\n");
> @@ -828,7 +828,7 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
>   			 LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
>   		drm_err(&dev_priv->drm, "Switching back to LCPLL failed\n");
>   
> -	snb_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> +	snb_pcode_write(&dev_priv->uncore, HSW_PCODE_DE_WRITE_FREQ_REQ,
>   			cdclk_config->voltage_level);
>   
>   	intel_de_write(dev_priv, CDCLK_FREQ,
> @@ -1086,7 +1086,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
>   	drm_WARN_ON_ONCE(&dev_priv->drm,
>   			 IS_SKYLAKE(dev_priv) && vco == 8640000);
>   
> -	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +	ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>   				SKL_CDCLK_PREPARE_FOR_CHANGE,
>   				SKL_CDCLK_READY_FOR_CHANGE,
>   				SKL_CDCLK_READY_FOR_CHANGE, 3);
> @@ -1132,7 +1132,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
>   	intel_de_posting_read(dev_priv, CDCLK_CTL);
>   
>   	/* inform PCU of the change */
> -	snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +	snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>   			cdclk_config->voltage_level);
>   
>   	intel_update_cdclk(dev_priv);
> @@ -1702,7 +1702,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   
>   	/* Inform power controller of upcoming frequency change. */
>   	if (DISPLAY_VER(dev_priv) >= 11)
> -		ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +		ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>   					SKL_CDCLK_PREPARE_FOR_CHANGE,
>   					SKL_CDCLK_READY_FOR_CHANGE,
>   					SKL_CDCLK_READY_FOR_CHANGE, 3);
> @@ -1711,7 +1711,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		 * BSpec requires us to wait up to 150usec, but that leads to
>   		 * timeouts; the 2ms used here is based on experiment.
>   		 */
> -		ret = snb_pcode_write_timeout(dev_priv,
> +		ret = snb_pcode_write_timeout(&dev_priv->uncore,
>   					      HSW_PCODE_DE_WRITE_FREQ_REQ,
>   					      0x80000000, 150, 2);
>   	if (ret) {
> @@ -1774,7 +1774,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe));
>   
>   	if (DISPLAY_VER(dev_priv) >= 11) {
> -		ret = snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>   				      cdclk_config->voltage_level);
>   	} else {
>   		/*
> @@ -1783,7 +1783,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		 * FIXME: Waiting for the request completion could be delayed
>   		 * until the next PCODE request based on BSpec.
>   		 */
> -		ret = snb_pcode_write_timeout(dev_priv,
> +		ret = snb_pcode_write_timeout(&dev_priv->uncore,
>   					      HSW_PCODE_DE_WRITE_FREQ_REQ,
>   					      cdclk_config->voltage_level,
>   					      150, 2);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 1d9bd5808849..74249da35281 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1194,7 +1194,7 @@ static u32 hsw_read_dcomp(struct drm_i915_private *dev_priv)
>   static void hsw_write_dcomp(struct drm_i915_private *dev_priv, u32 val)
>   {
>   	if (IS_HASWELL(dev_priv)) {
> -		if (snb_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, val))
> +		if (snb_pcode_write(&dev_priv->uncore, GEN6_PCODE_WRITE_D_COMP, val))
>   			drm_dbg_kms(&dev_priv->drm,
>   				    "Failed to write to D_COMP\n");
>   	} else {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> index 5be18eb94042..91cfd5890f46 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -474,7 +474,7 @@ static void icl_tc_cold_exit(struct drm_i915_private *i915)
>   	int ret, tries = 0;
>   
>   	while (1) {
> -		ret = snb_pcode_write_timeout(i915, ICL_PCODE_EXIT_TCCOLD, 0,
> +		ret = snb_pcode_write_timeout(&i915->uncore, ICL_PCODE_EXIT_TCCOLD, 0,
>   					      250, 1);
>   		if (ret != -EAGAIN || ++tries == 3)
>   			break;
> @@ -1739,7 +1739,7 @@ tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
>   		 * Spec states that we should timeout the request after 200us
>   		 * but the function below will timeout after 500us
>   		 */
> -		ret = snb_pcode_read(i915, TGL_PCODE_TCCOLD, &low_val, &high_val);
> +		ret = snb_pcode_read(&i915->uncore, TGL_PCODE_TCCOLD, &low_val, &high_val);
>   		if (ret == 0) {
>   			if (block &&
>   			    (low_val & TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 44ac0cee8b77..8ea66a2e1b09 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -298,7 +298,7 @@ static int intel_hdcp_load_keys(struct drm_i915_private *dev_priv)
>   	 * Mailbox interface.
>   	 */
>   	if (DISPLAY_VER(dev_priv) == 9 && !IS_BROXTON(dev_priv)) {
> -		ret = snb_pcode_write(dev_priv, SKL_PCODE_LOAD_HDCP_KEYS, 1);
> +		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_LOAD_HDCP_KEYS, 1);
>   		if (ret) {
>   			drm_err(&dev_priv->drm,
>   				"Failed to initiate HDCP key load (%d)\n",
> 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 0c6b9eb724ae..90a440865037 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -138,7 +138,7 @@ static int gen6_drpc(struct seq_file *m)
>   	}
>   
>   	if (GRAPHICS_VER(i915) <= 7)
> -		snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
> +		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
>   
>   	seq_printf(m, "RC1e Enabled: %s\n",
>   		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC1e_ENABLE));
> @@ -545,7 +545,7 @@ static int llc_show(struct seq_file *m, void *data)
>   	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
>   	for (gpu_freq = min_gpu_freq; gpu_freq <= max_gpu_freq; gpu_freq++) {
>   		ia_freq = gpu_freq;
> -		snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
> +		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
>   			       &ia_freq, NULL);
>   		seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
>   			   intel_gpu_freq(rps,
> diff --git a/drivers/gpu/drm/i915/gt/intel_llc.c b/drivers/gpu/drm/i915/gt/intel_llc.c
> index 40e2e28ee6c7..14fe65812e42 100644
> --- a/drivers/gpu/drm/i915/gt/intel_llc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_llc.c
> @@ -124,7 +124,6 @@ static void calc_ia_freq(struct intel_llc *llc,
>   
>   static void gen6_update_ring_freq(struct intel_llc *llc)
>   {
> -	struct drm_i915_private *i915 = llc_to_gt(llc)->i915;
>   	struct ia_constants consts;
>   	unsigned int gpu_freq;
>   
> @@ -142,7 +141,7 @@ static void gen6_update_ring_freq(struct intel_llc *llc)
>   		unsigned int ia_freq, ring_freq;
>   
>   		calc_ia_freq(llc, gpu_freq, &consts, &ia_freq, &ring_freq);
> -		snb_pcode_write(i915, GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
> +		snb_pcode_write(llc_to_gt(llc)->uncore, GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
>   				ia_freq << GEN6_PCODE_FREQ_IA_RATIO_SHIFT |
>   				ring_freq << GEN6_PCODE_FREQ_RING_RATIO_SHIFT |
>   				gpu_freq);
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index b4770690e794..f8d0523f4c18 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -272,7 +272,7 @@ static void gen6_rc6_enable(struct intel_rc6 *rc6)
>   	    GEN6_RC_CTL_HW_ENABLE;
>   
>   	rc6vids = 0;
> -	ret = snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
> +	ret = snb_pcode_read(rc6_to_gt(rc6)->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
>   	if (GRAPHICS_VER(i915) == 6 && ret) {
>   		drm_dbg(&i915->drm, "Couldn't check for BIOS workaround\n");
>   	} else if (GRAPHICS_VER(i915) == 6 &&
> @@ -282,7 +282,7 @@ static void gen6_rc6_enable(struct intel_rc6 *rc6)
>   			GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
>   		rc6vids &= 0xffff00;
>   		rc6vids |= GEN6_ENCODE_RC6_VID(450);
> -		ret = snb_pcode_write(i915, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
> +		ret = snb_pcode_write(rc6_to_gt(rc6)->uncore, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
>   		if (ret)
>   			drm_err(&i915->drm,
>   				"Couldn't fix incorrect rc6 voltage\n");
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 3bd8415a0f1b..a62d323ff056 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1144,7 +1144,7 @@ static void gen6_rps_init(struct intel_rps *rps)
>   
>   		if (IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11)
>   			mult = GEN9_FREQ_SCALER;
> -		if (snb_pcode_read(i915, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> +		if (snb_pcode_read(rps_to_gt(rps)->uncore, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
>   				   &ddcc_status, NULL) == 0)
>   			rps->efficient_freq =
>   				clamp_t(u32,
> @@ -1984,7 +1984,7 @@ void intel_rps_init(struct intel_rps *rps)
>   	if (GRAPHICS_VER(i915) == 6 || IS_IVYBRIDGE(i915) || IS_HASWELL(i915)) {
>   		u32 params = 0;
>   
> -		snb_pcode_read(i915, GEN6_READ_OC_PARAMS, &params, NULL);
> +		snb_pcode_read(rps_to_gt(rps)->uncore, GEN6_READ_OC_PARAMS, &params, NULL);
>   		if (params & BIT(31)) { /* OC supported */
>   			drm_dbg(&i915->drm,
>   				"Overclocking supported, max: %dMHz, overclock: %dMHz\n",
> diff --git a/drivers/gpu/drm/i915/gt/selftest_llc.c b/drivers/gpu/drm/i915/gt/selftest_llc.c
> index 2cd184ab32b1..cfd736d88939 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_llc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_llc.c
> @@ -31,7 +31,7 @@ static int gen6_verify_ring_freq(struct intel_llc *llc)
>   		calc_ia_freq(llc, gpu_freq, &consts, &ia_freq, &ring_freq);
>   
>   		val = gpu_freq;
> -		if (snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
> +		if (snb_pcode_read(llc_to_gt(llc)->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
>   				   &val, NULL)) {
>   			pr_err("Failed to read freq table[%d], range [%d, %d]\n",
>   			       gpu_freq, consts.min_gpu_freq, consts.max_gpu_freq);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index 6a69ac0184ad..cfb4708dd62e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -521,7 +521,7 @@ static void show_pcu_config(struct intel_rps *rps)
>   	for (gpu_freq = min_gpu_freq; gpu_freq <= max_gpu_freq; gpu_freq++) {
>   		int ia_freq = gpu_freq;
>   
> -		snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
> +		snb_pcode_read(rps_to_gt(rps)->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
>   			       &ia_freq, NULL);
>   
>   		pr_info("%5d  %5d  %5d\n",
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 90b0ce5051af..0e9763868d68 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -629,7 +629,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>   
>   	intel_opregion_setup(dev_priv);
>   
> -	ret = intel_pcode_init(dev_priv);
> +	ret = intel_pcode_init(&dev_priv->uncore);
>   	if (ret)
>   		goto err_msi;
>   
> @@ -1251,7 +1251,7 @@ static int i915_drm_resume(struct drm_device *dev)
>   
>   	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
> -	ret = intel_pcode_init(dev_priv);
> +	ret = intel_pcode_init(&dev_priv->uncore);
>   	if (ret)
>   		return ret;
>   
> diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
> index 2b9e7833da96..437447119770 100644
> --- a/drivers/gpu/drm/i915/intel_dram.c
> +++ b/drivers/gpu/drm/i915/intel_dram.c
> @@ -393,7 +393,7 @@ static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
>   	u32 val = 0;
>   	int ret;
>   
> -	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>   			     ICL_PCODE_MEM_SS_READ_GLOBAL_INFO, &val, NULL);
>   	if (ret)
>   		return ret;
> diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c
> index ac727546868e..2be700932322 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.c
> +++ b/drivers/gpu/drm/i915/intel_pcode.c
> @@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox)
>   	}
>   }
>   
> -static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
> +static int __snb_pcode_rw(struct intel_uncore *uncore, u32 mbox,
>   			  u32 *val, u32 *val1,
>   			  int fast_timeout_us, int slow_timeout_ms,
>   			  bool is_read)
>   {
> -	struct intel_uncore *uncore = &i915->uncore;
> -
> -	lockdep_assert_held(&i915->sb_lock);
> +	lockdep_assert_held(&uncore->i915->sb_lock);
>   
>   	/*
>   	 * GEN6_PCODE_* are outside of the forcewake domain, we can use
> @@ -88,22 +86,22 @@ static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
>   	if (is_read && val1)
>   		*val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
>   
> -	if (GRAPHICS_VER(i915) > 6)
> +	if (GRAPHICS_VER(uncore->i915) > 6)
>   		return gen7_check_mailbox_status(mbox);
>   	else
>   		return gen6_check_mailbox_status(mbox);
>   }
>   
> -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1)
> +int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1)
>   {
>   	int err;
>   
> -	mutex_lock(&i915->sb_lock);
> -	err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
> -	mutex_unlock(&i915->sb_lock);
> +	mutex_lock(&uncore->i915->sb_lock);
> +	err = __snb_pcode_rw(uncore, mbox, val, val1, 500, 20, true);
> +	mutex_unlock(&uncore->i915->sb_lock);
>   
>   	if (err) {
> -		drm_dbg(&i915->drm,
> +		drm_dbg(&uncore->i915->drm,
>   			"warning: pcode (read from mbox %x) mailbox access failed for %ps: %d\n",
>   			mbox, __builtin_return_address(0), err);
>   	}
> @@ -111,18 +109,18 @@ int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1)
>   	return err;
>   }
>   
> -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
> +int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val,
>   			    int fast_timeout_us, int slow_timeout_ms)
>   {
>   	int err;
>   
> -	mutex_lock(&i915->sb_lock);
> -	err = __snb_pcode_rw(i915, mbox, &val, NULL,
> +	mutex_lock(&uncore->i915->sb_lock);
> +	err = __snb_pcode_rw(uncore, mbox, &val, NULL,
>   			     fast_timeout_us, slow_timeout_ms, false);
> -	mutex_unlock(&i915->sb_lock);
> +	mutex_unlock(&uncore->i915->sb_lock);
>   
>   	if (err) {
> -		drm_dbg(&i915->drm,
> +		drm_dbg(&uncore->i915->drm,
>   			"warning: pcode (write of 0x%08x to mbox %x) mailbox access failed for %ps: %d\n",
>   			val, mbox, __builtin_return_address(0), err);
>   	}
> @@ -130,18 +128,18 @@ int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
>   	return err;
>   }
>   
> -static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> +static bool skl_pcode_try_request(struct intel_uncore *uncore, u32 mbox,
>   				  u32 request, u32 reply_mask, u32 reply,
>   				  u32 *status)
>   {
> -	*status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
> +	*status = __snb_pcode_rw(uncore, mbox, &request, NULL, 500, 0, true);
>   
>   	return (*status == 0) && ((request & reply_mask) == reply);
>   }
>   
>   /**
>    * skl_pcode_request - send PCODE request until acknowledgment
> - * @i915: device private
> + * @uncore: uncore
>    * @mbox: PCODE mailbox ID the request is targeted for
>    * @request: request ID
>    * @reply_mask: mask used to check for request acknowledgment
> @@ -158,16 +156,16 @@ static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
>    * Returns 0 on success, %-ETIMEDOUT in case of a timeout, <0 in case of some
>    * other error as reported by PCODE.
>    */
> -int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> +int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
>   		      u32 reply_mask, u32 reply, int timeout_base_ms)
>   {
>   	u32 status;
>   	int ret;
>   
> -	mutex_lock(&i915->sb_lock);
> +	mutex_lock(&uncore->i915->sb_lock);
>   
>   #define COND \
> -	skl_pcode_try_request(i915, mbox, request, reply_mask, reply, &status)
> +	skl_pcode_try_request(uncore, mbox, request, reply_mask, reply, &status)
>   
>   	/*
>   	 * Prime the PCODE by doing a request first. Normally it guarantees
> @@ -193,35 +191,26 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
>   	 * requests, and for any quirks of the PCODE firmware that delays
>   	 * the request completion.
>   	 */
> -	drm_dbg_kms(&i915->drm,
> +	drm_dbg_kms(&uncore->i915->drm,
>   		    "PCODE timeout, retrying with preemption disabled\n");
> -	drm_WARN_ON_ONCE(&i915->drm, timeout_base_ms > 3);
> +	drm_WARN_ON_ONCE(&uncore->i915->drm, timeout_base_ms > 3);
>   	preempt_disable();
>   	ret = wait_for_atomic(COND, 50);
>   	preempt_enable();
>   
>   out:
> -	mutex_unlock(&i915->sb_lock);
> +	mutex_unlock(&uncore->i915->sb_lock);
>   	return status ? status : ret;
>   #undef COND
>   }
>   
> -int intel_pcode_init(struct drm_i915_private *i915)
> +int intel_pcode_init(struct intel_uncore *uncore)
>   {
> -	int ret = 0;
> -
> -	if (!IS_DGFX(i915))
> -		return ret;
> -
> -	ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
> -				DG1_UNCORE_GET_INIT_STATUS,
> -				DG1_UNCORE_INIT_STATUS_COMPLETE,
> -				DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
> -
> -	drm_dbg(&i915->drm, "PCODE init status %d\n", ret);
> -
> -	if (ret)
> -		drm_err(&i915->drm, "Pcode did not report uncore initialization completion!\n");

Strictly speaking you would leave the error messages in here for the 
next patch to remove them. Otherwise at this point in the series, if it 
is half-merged, there wouldn't be any logging left. But doesn't matter 
in this case since we don't expect series to be part merged. Only if 
someone will be bisecting PCODE errors during module load it can 
theoretically come into play but it's unlikely.

Regards,

Tvrtko

> +	if (!IS_DGFX(uncore->i915))
> +		return 0;
>   
> -	return ret;
> +	return skl_pcode_request(uncore, DG1_PCODE_STATUS,
> +				 DG1_UNCORE_GET_INIT_STATUS,
> +				 DG1_UNCORE_INIT_STATUS_COMPLETE,
> +				 DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_pcode.h b/drivers/gpu/drm/i915/intel_pcode.h
> index 0962a17fac48..8f6241b114a5 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.h
> +++ b/drivers/gpu/drm/i915/intel_pcode.h
> @@ -8,17 +8,17 @@
>   
>   #include <linux/types.h>
>   
> -struct drm_i915_private;
> +struct intel_uncore;
>   
> -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1);
> -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
> +int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1);
> +int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val,
>   			    int fast_timeout_us, int slow_timeout_ms);
> -#define snb_pcode_write(i915, mbox, val)			\
> -	snb_pcode_write_timeout(i915, mbox, val, 500, 0)
> +#define snb_pcode_write(uncore, mbox, val) \
> +	snb_pcode_write_timeout(uncore, mbox, val, 500, 0)
>   
> -int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> +int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
>   		      u32 reply_mask, u32 reply, int timeout_base_ms);
>   
> -int intel_pcode_init(struct drm_i915_private *i915);
> +int intel_pcode_init(struct intel_uncore *uncore);
>   
>   #endif /* _INTEL_PCODE_H */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ee0047fdc95d..aacb21cbc62e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2874,7 +2874,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
>   
>   		/* read the first set of memory latencies[0:3] */
>   		val = 0; /* data0 to be programmed to 0 for first set */
> -		ret = snb_pcode_read(dev_priv, GEN9_PCODE_READ_MEM_LATENCY,
> +		ret = snb_pcode_read(&dev_priv->uncore, GEN9_PCODE_READ_MEM_LATENCY,
>   				     &val, NULL);
>   
>   		if (ret) {
> @@ -2893,7 +2893,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
>   
>   		/* read the second set of memory latencies[4:7] */
>   		val = 1; /* data0 to be programmed to 1 for second set */
> -		ret = snb_pcode_read(dev_priv, GEN9_PCODE_READ_MEM_LATENCY,
> +		ret = snb_pcode_read(&dev_priv->uncore, GEN9_PCODE_READ_MEM_LATENCY,
>   				     &val, NULL);
>   		if (ret) {
>   			drm_err(&dev_priv->drm,
> @@ -3679,7 +3679,7 @@ intel_sagv_block_time(struct drm_i915_private *dev_priv)
>   		u32 val = 0;
>   		int ret;
>   
> -		ret = snb_pcode_read(dev_priv,
> +		ret = snb_pcode_read(&dev_priv->uncore,
>   				     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
>   				     &val, NULL);
>   		if (ret) {
> @@ -3748,7 +3748,7 @@ static void skl_sagv_enable(struct drm_i915_private *dev_priv)
>   		return;
>   
>   	drm_dbg_kms(&dev_priv->drm, "Enabling SAGV\n");
> -	ret = snb_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,
> +	ret = snb_pcode_write(&dev_priv->uncore, GEN9_PCODE_SAGV_CONTROL,
>   			      GEN9_SAGV_ENABLE);
>   
>   	/* We don't need to wait for SAGV when enabling */
> @@ -3781,7 +3781,7 @@ static void skl_sagv_disable(struct drm_i915_private *dev_priv)
>   
>   	drm_dbg_kms(&dev_priv->drm, "Disabling SAGV\n");
>   	/* bspec says to keep retrying for at least 1 ms */
> -	ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL,
> +	ret = skl_pcode_request(&dev_priv->uncore, GEN9_PCODE_SAGV_CONTROL,
>   				GEN9_SAGV_DISABLE,
>   				GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
>   				1);

Re: [Intel-gfx] [PATCH v5 0/7] drm/i915: Media freq factor and per-gt enhancements/fixes

[Tvrtko Ursulin]

On 12/05/2022 05:38, Dixit, Ashutosh wrote:
> On Wed, 11 May 2022 19:32:13 -0700, Ashutosh Dixit wrote:
>>
>> Some recent Intel dGfx platforms allow media IP to work at a different
>> frequency from the base GT. This patch series exposes sysfs controls for
>> this functionality in the new per-gt sysfs. Some enhancements and fixes to
>> previous per-gt functionality are also included to complete the new
>> functionality:
>> * Patches 1 and 2 implement basic sysfs controls for media freq
>> * Patch 3 extends previous pcode functions for multiple gt's
>> * Patch 4 inits pcode on different gt's
>> * Patch 5 adds a couple of pcode helpers
>> * Patch 6 uses the new pcode functions to retrieve media RP0/RPn freq
>> * Patch 7 fixes memory leaks in the previous per-gt sysfs implementation
>>    and some code refactoring
> 
> In this v5 I have dropped the last two patches of the v4 series, these
> ones:
> 
> [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs
> [PATCH 8/8] drm/i915/gt: Expose default value for media_freq_factor in per-gt sysfs
> 
> Because these need more work based on the review comments. If this series
> is merged I will submit these patches as a separate series, otherwise I
> will re-add them to this series and resubmit (due to dependence between
> this series and those patches).
> 
> Apart from this, I believe I have addressed all previous review comments on
> the patches in this series.
> 
> Thanks for reviewing,

Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

At some point we should starting documenting our sysfs in 
Documentation/ABI. For instance the freq ratio this patch adds really 
does need some user facing docs to know how to use it. Would you sign up 
to document the bits this series adds as follow up?

Regards,

Tvrtko

Re: [Intel-gfx] [PATCH 3/7] drm/i915/pcode: Extend pcode functions for multiple gt's

[Jani Nikula]

On Wed, 11 May 2022, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> Each gt contains an independent instance of pcode. Extend pcode functions
> to interface with pcode on different gt's. To avoid creating dependency of
> display functionality on intel_gt, pcode function interfaces are exposed in
> terms of uncore rather than intel_gt. Callers have been converted to pass
> in the appropritate (i915 or intel_gt) uncore to the pcode functions.
>
> v2: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
> v3: Retain previous function names to eliminate needless #defines (Rodrigo)
> v4: Move out i915_pcode_init() to a separate patch (Tvrtko)
>     Remove duplicated drm_err/drm_dbg from intel_pcode_init() (Tvrtko)

Couple of nitpicks inline, and not insisting on changing. Basically ack
on this from me.

>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/hsw_ips.c        |  4 +-
>  drivers/gpu/drm/i915/display/intel_bw.c       |  6 +-
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 16 ++---
>  .../drm/i915/display/intel_display_power.c    |  2 +-
>  .../i915/display/intel_display_power_well.c   |  4 +-
>  drivers/gpu/drm/i915/display/intel_hdcp.c     |  2 +-
>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |  4 +-
>  drivers/gpu/drm/i915/gt/intel_llc.c           |  3 +-
>  drivers/gpu/drm/i915/gt/intel_rc6.c           |  4 +-
>  drivers/gpu/drm/i915/gt/intel_rps.c           |  4 +-
>  drivers/gpu/drm/i915/gt/selftest_llc.c        |  2 +-
>  drivers/gpu/drm/i915/gt/selftest_rps.c        |  2 +-
>  drivers/gpu/drm/i915/i915_driver.c            |  4 +-
>  drivers/gpu/drm/i915/intel_dram.c             |  2 +-
>  drivers/gpu/drm/i915/intel_pcode.c            | 69 ++++++++-----------
>  drivers/gpu/drm/i915/intel_pcode.h            | 14 ++--
>  drivers/gpu/drm/i915/intel_pm.c               | 10 +--
>  17 files changed, 70 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/hsw_ips.c b/drivers/gpu/drm/i915/display/hsw_ips.c
> index 38014e0cc9ad..861dcd2eb890 100644
> --- a/drivers/gpu/drm/i915/display/hsw_ips.c
> +++ b/drivers/gpu/drm/i915/display/hsw_ips.c
> @@ -28,7 +28,7 @@ static void hsw_ips_enable(const struct intel_crtc_state *crtc_state)
>  
>  	if (IS_BROADWELL(i915)) {
>  		drm_WARN_ON(&i915->drm,
> -			    snb_pcode_write(i915, DISPLAY_IPS_CONTROL,
> +			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL,
>  					    IPS_ENABLE | IPS_PCODE_CONTROL));
>  		/*
>  		 * Quoting Art Runyan: "its not safe to expect any particular
> @@ -62,7 +62,7 @@ bool hsw_ips_disable(const struct intel_crtc_state *crtc_state)
>  
>  	if (IS_BROADWELL(i915)) {
>  		drm_WARN_ON(&i915->drm,
> -			    snb_pcode_write(i915, DISPLAY_IPS_CONTROL, 0));
> +			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL, 0));
>  		/*
>  		 * Wait for PCODE to finish disabling IPS. The BSpec specified
>  		 * 42ms timeout value leads to occasional timeouts so use 100ms
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 37bd7b17f3d0..79269d2c476b 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -78,7 +78,7 @@ static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>  	u16 dclk;
>  	int ret;
>  
> -	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>  			     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
>  			     &val, &val2);
>  	if (ret)
> @@ -104,7 +104,7 @@ static int adls_pcode_read_psf_gv_point_info(struct drm_i915_private *dev_priv,
>  	int ret;
>  	int i;
>  
> -	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>  			     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO, &val, NULL);
>  	if (ret)
>  		return ret;
> @@ -123,7 +123,7 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
>  	int ret;
>  
>  	/* bspec says to keep retrying for at least 1 ms */
> -	ret = skl_pcode_request(dev_priv, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
> +	ret = skl_pcode_request(&dev_priv->uncore, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
>  				points_mask,
>  				ICL_PCODE_REP_QGV_MASK | ADLS_PCODE_REP_PSF_MASK,
>  				ICL_PCODE_REP_QGV_SAFE | ADLS_PCODE_REP_PSF_SAFE,
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index b2017d8161b4..6e80162632dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -800,7 +800,7 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
>  		     "trying to change cdclk frequency with cdclk not enabled\n"))
>  		return;
>  
> -	ret = snb_pcode_write(dev_priv, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> +	ret = snb_pcode_write(&dev_priv->uncore, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
>  	if (ret) {
>  		drm_err(&dev_priv->drm,
>  			"failed to inform pcode about cdclk change\n");
> @@ -828,7 +828,7 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
>  			 LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
>  		drm_err(&dev_priv->drm, "Switching back to LCPLL failed\n");
>  
> -	snb_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> +	snb_pcode_write(&dev_priv->uncore, HSW_PCODE_DE_WRITE_FREQ_REQ,
>  			cdclk_config->voltage_level);
>  
>  	intel_de_write(dev_priv, CDCLK_FREQ,
> @@ -1086,7 +1086,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
>  	drm_WARN_ON_ONCE(&dev_priv->drm,
>  			 IS_SKYLAKE(dev_priv) && vco == 8640000);
>  
> -	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +	ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>  				SKL_CDCLK_PREPARE_FOR_CHANGE,
>  				SKL_CDCLK_READY_FOR_CHANGE,
>  				SKL_CDCLK_READY_FOR_CHANGE, 3);
> @@ -1132,7 +1132,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
>  	intel_de_posting_read(dev_priv, CDCLK_CTL);
>  
>  	/* inform PCU of the change */
> -	snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +	snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>  			cdclk_config->voltage_level);
>  
>  	intel_update_cdclk(dev_priv);
> @@ -1702,7 +1702,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  
>  	/* Inform power controller of upcoming frequency change. */
>  	if (DISPLAY_VER(dev_priv) >= 11)
> -		ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +		ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>  					SKL_CDCLK_PREPARE_FOR_CHANGE,
>  					SKL_CDCLK_READY_FOR_CHANGE,
>  					SKL_CDCLK_READY_FOR_CHANGE, 3);
> @@ -1711,7 +1711,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  		 * BSpec requires us to wait up to 150usec, but that leads to
>  		 * timeouts; the 2ms used here is based on experiment.
>  		 */
> -		ret = snb_pcode_write_timeout(dev_priv,
> +		ret = snb_pcode_write_timeout(&dev_priv->uncore,
>  					      HSW_PCODE_DE_WRITE_FREQ_REQ,
>  					      0x80000000, 150, 2);
>  	if (ret) {
> @@ -1774,7 +1774,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  		intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe));
>  
>  	if (DISPLAY_VER(dev_priv) >= 11) {
> -		ret = snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
>  				      cdclk_config->voltage_level);
>  	} else {
>  		/*
> @@ -1783,7 +1783,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  		 * FIXME: Waiting for the request completion could be delayed
>  		 * until the next PCODE request based on BSpec.
>  		 */
> -		ret = snb_pcode_write_timeout(dev_priv,
> +		ret = snb_pcode_write_timeout(&dev_priv->uncore,
>  					      HSW_PCODE_DE_WRITE_FREQ_REQ,
>  					      cdclk_config->voltage_level,
>  					      150, 2);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 1d9bd5808849..74249da35281 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1194,7 +1194,7 @@ static u32 hsw_read_dcomp(struct drm_i915_private *dev_priv)
>  static void hsw_write_dcomp(struct drm_i915_private *dev_priv, u32 val)
>  {
>  	if (IS_HASWELL(dev_priv)) {
> -		if (snb_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, val))
> +		if (snb_pcode_write(&dev_priv->uncore, GEN6_PCODE_WRITE_D_COMP, val))
>  			drm_dbg_kms(&dev_priv->drm,
>  				    "Failed to write to D_COMP\n");
>  	} else {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> index 5be18eb94042..91cfd5890f46 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -474,7 +474,7 @@ static void icl_tc_cold_exit(struct drm_i915_private *i915)
>  	int ret, tries = 0;
>  
>  	while (1) {
> -		ret = snb_pcode_write_timeout(i915, ICL_PCODE_EXIT_TCCOLD, 0,
> +		ret = snb_pcode_write_timeout(&i915->uncore, ICL_PCODE_EXIT_TCCOLD, 0,
>  					      250, 1);
>  		if (ret != -EAGAIN || ++tries == 3)
>  			break;
> @@ -1739,7 +1739,7 @@ tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
>  		 * Spec states that we should timeout the request after 200us
>  		 * but the function below will timeout after 500us
>  		 */
> -		ret = snb_pcode_read(i915, TGL_PCODE_TCCOLD, &low_val, &high_val);
> +		ret = snb_pcode_read(&i915->uncore, TGL_PCODE_TCCOLD, &low_val, &high_val);
>  		if (ret == 0) {
>  			if (block &&
>  			    (low_val & TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 44ac0cee8b77..8ea66a2e1b09 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -298,7 +298,7 @@ static int intel_hdcp_load_keys(struct drm_i915_private *dev_priv)
>  	 * Mailbox interface.
>  	 */
>  	if (DISPLAY_VER(dev_priv) == 9 && !IS_BROXTON(dev_priv)) {
> -		ret = snb_pcode_write(dev_priv, SKL_PCODE_LOAD_HDCP_KEYS, 1);
> +		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_LOAD_HDCP_KEYS, 1);
>  		if (ret) {
>  			drm_err(&dev_priv->drm,
>  				"Failed to initiate HDCP key load (%d)\n",
> 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 0c6b9eb724ae..90a440865037 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -138,7 +138,7 @@ static int gen6_drpc(struct seq_file *m)
>  	}
>  
>  	if (GRAPHICS_VER(i915) <= 7)
> -		snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
> +		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);

Pedantically, I'm wondering if this (and similar places) should first be
i915->uncore, to be replaced with gt->uncore in the next patch.

>  
>  	seq_printf(m, "RC1e Enabled: %s\n",
>  		   str_yes_no(rcctl1 & GEN6_RC_CTL_RC1e_ENABLE));
> @@ -545,7 +545,7 @@ static int llc_show(struct seq_file *m, void *data)
>  	wakeref = intel_runtime_pm_get(gt->uncore->rpm);
>  	for (gpu_freq = min_gpu_freq; gpu_freq <= max_gpu_freq; gpu_freq++) {
>  		ia_freq = gpu_freq;
> -		snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
> +		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
>  			       &ia_freq, NULL);
>  		seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
>  			   intel_gpu_freq(rps,
> diff --git a/drivers/gpu/drm/i915/gt/intel_llc.c b/drivers/gpu/drm/i915/gt/intel_llc.c
> index 40e2e28ee6c7..14fe65812e42 100644
> --- a/drivers/gpu/drm/i915/gt/intel_llc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_llc.c
> @@ -124,7 +124,6 @@ static void calc_ia_freq(struct intel_llc *llc,
>  
>  static void gen6_update_ring_freq(struct intel_llc *llc)
>  {
> -	struct drm_i915_private *i915 = llc_to_gt(llc)->i915;
>  	struct ia_constants consts;
>  	unsigned int gpu_freq;
>  
> @@ -142,7 +141,7 @@ static void gen6_update_ring_freq(struct intel_llc *llc)
>  		unsigned int ia_freq, ring_freq;
>  
>  		calc_ia_freq(llc, gpu_freq, &consts, &ia_freq, &ring_freq);
> -		snb_pcode_write(i915, GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
> +		snb_pcode_write(llc_to_gt(llc)->uncore, GEN6_PCODE_WRITE_MIN_FREQ_TABLE,
>  				ia_freq << GEN6_PCODE_FREQ_IA_RATIO_SHIFT |
>  				ring_freq << GEN6_PCODE_FREQ_RING_RATIO_SHIFT |
>  				gpu_freq);
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index b4770690e794..f8d0523f4c18 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -272,7 +272,7 @@ static void gen6_rc6_enable(struct intel_rc6 *rc6)
>  	    GEN6_RC_CTL_HW_ENABLE;
>  
>  	rc6vids = 0;
> -	ret = snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
> +	ret = snb_pcode_read(rc6_to_gt(rc6)->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
>  	if (GRAPHICS_VER(i915) == 6 && ret) {
>  		drm_dbg(&i915->drm, "Couldn't check for BIOS workaround\n");
>  	} else if (GRAPHICS_VER(i915) == 6 &&
> @@ -282,7 +282,7 @@ static void gen6_rc6_enable(struct intel_rc6 *rc6)
>  			GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
>  		rc6vids &= 0xffff00;
>  		rc6vids |= GEN6_ENCODE_RC6_VID(450);
> -		ret = snb_pcode_write(i915, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
> +		ret = snb_pcode_write(rc6_to_gt(rc6)->uncore, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
>  		if (ret)
>  			drm_err(&i915->drm,
>  				"Couldn't fix incorrect rc6 voltage\n");
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 3bd8415a0f1b..a62d323ff056 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1144,7 +1144,7 @@ static void gen6_rps_init(struct intel_rps *rps)
>  
>  		if (IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11)
>  			mult = GEN9_FREQ_SCALER;
> -		if (snb_pcode_read(i915, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> +		if (snb_pcode_read(rps_to_gt(rps)->uncore, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
>  				   &ddcc_status, NULL) == 0)
>  			rps->efficient_freq =
>  				clamp_t(u32,
> @@ -1984,7 +1984,7 @@ void intel_rps_init(struct intel_rps *rps)
>  	if (GRAPHICS_VER(i915) == 6 || IS_IVYBRIDGE(i915) || IS_HASWELL(i915)) {
>  		u32 params = 0;
>  
> -		snb_pcode_read(i915, GEN6_READ_OC_PARAMS, &params, NULL);
> +		snb_pcode_read(rps_to_gt(rps)->uncore, GEN6_READ_OC_PARAMS, &params, NULL);
>  		if (params & BIT(31)) { /* OC supported */
>  			drm_dbg(&i915->drm,
>  				"Overclocking supported, max: %dMHz, overclock: %dMHz\n",
> diff --git a/drivers/gpu/drm/i915/gt/selftest_llc.c b/drivers/gpu/drm/i915/gt/selftest_llc.c
> index 2cd184ab32b1..cfd736d88939 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_llc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_llc.c
> @@ -31,7 +31,7 @@ static int gen6_verify_ring_freq(struct intel_llc *llc)
>  		calc_ia_freq(llc, gpu_freq, &consts, &ia_freq, &ring_freq);
>  
>  		val = gpu_freq;
> -		if (snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
> +		if (snb_pcode_read(llc_to_gt(llc)->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
>  				   &val, NULL)) {
>  			pr_err("Failed to read freq table[%d], range [%d, %d]\n",
>  			       gpu_freq, consts.min_gpu_freq, consts.max_gpu_freq);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index 6a69ac0184ad..cfb4708dd62e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -521,7 +521,7 @@ static void show_pcu_config(struct intel_rps *rps)
>  	for (gpu_freq = min_gpu_freq; gpu_freq <= max_gpu_freq; gpu_freq++) {
>  		int ia_freq = gpu_freq;
>  
> -		snb_pcode_read(i915, GEN6_PCODE_READ_MIN_FREQ_TABLE,
> +		snb_pcode_read(rps_to_gt(rps)->uncore, GEN6_PCODE_READ_MIN_FREQ_TABLE,
>  			       &ia_freq, NULL);
>  
>  		pr_info("%5d  %5d  %5d\n",
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 90b0ce5051af..0e9763868d68 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -629,7 +629,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>  
>  	intel_opregion_setup(dev_priv);
>  
> -	ret = intel_pcode_init(dev_priv);
> +	ret = intel_pcode_init(&dev_priv->uncore);
>  	if (ret)
>  		goto err_msi;
>  
> @@ -1251,7 +1251,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
> -	ret = intel_pcode_init(dev_priv);
> +	ret = intel_pcode_init(&dev_priv->uncore);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
> index 2b9e7833da96..437447119770 100644
> --- a/drivers/gpu/drm/i915/intel_dram.c
> +++ b/drivers/gpu/drm/i915/intel_dram.c
> @@ -393,7 +393,7 @@ static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
>  	u32 val = 0;
>  	int ret;
>  
> -	ret = snb_pcode_read(dev_priv, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> +	ret = snb_pcode_read(&dev_priv->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>  			     ICL_PCODE_MEM_SS_READ_GLOBAL_INFO, &val, NULL);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c
> index ac727546868e..2be700932322 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.c
> +++ b/drivers/gpu/drm/i915/intel_pcode.c
> @@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox)
>  	}
>  }
>  
> -static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
> +static int __snb_pcode_rw(struct intel_uncore *uncore, u32 mbox,
>  			  u32 *val, u32 *val1,
>  			  int fast_timeout_us, int slow_timeout_ms,
>  			  bool is_read)
>  {
> -	struct intel_uncore *uncore = &i915->uncore;

Nitpick, personally, I would probably have just replaced the above with

	struct drm_i915_private *i915 = uncore->i915;

to minimize the diff. Ditto everywhere. But not a big deal.

BR,
Jani.

> -
> -	lockdep_assert_held(&i915->sb_lock);
> +	lockdep_assert_held(&uncore->i915->sb_lock);
>  
>  	/*
>  	 * GEN6_PCODE_* are outside of the forcewake domain, we can use
> @@ -88,22 +86,22 @@ static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
>  	if (is_read && val1)
>  		*val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
>  
> -	if (GRAPHICS_VER(i915) > 6)
> +	if (GRAPHICS_VER(uncore->i915) > 6)
>  		return gen7_check_mailbox_status(mbox);
>  	else
>  		return gen6_check_mailbox_status(mbox);
>  }
>  
> -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1)
> +int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1)
>  {
>  	int err;
>  
> -	mutex_lock(&i915->sb_lock);
> -	err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
> -	mutex_unlock(&i915->sb_lock);
> +	mutex_lock(&uncore->i915->sb_lock);
> +	err = __snb_pcode_rw(uncore, mbox, val, val1, 500, 20, true);
> +	mutex_unlock(&uncore->i915->sb_lock);
>  
>  	if (err) {
> -		drm_dbg(&i915->drm,
> +		drm_dbg(&uncore->i915->drm,
>  			"warning: pcode (read from mbox %x) mailbox access failed for %ps: %d\n",
>  			mbox, __builtin_return_address(0), err);
>  	}
> @@ -111,18 +109,18 @@ int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1)
>  	return err;
>  }
>  
> -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
> +int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val,
>  			    int fast_timeout_us, int slow_timeout_ms)
>  {
>  	int err;
>  
> -	mutex_lock(&i915->sb_lock);
> -	err = __snb_pcode_rw(i915, mbox, &val, NULL,
> +	mutex_lock(&uncore->i915->sb_lock);
> +	err = __snb_pcode_rw(uncore, mbox, &val, NULL,
>  			     fast_timeout_us, slow_timeout_ms, false);
> -	mutex_unlock(&i915->sb_lock);
> +	mutex_unlock(&uncore->i915->sb_lock);
>  
>  	if (err) {
> -		drm_dbg(&i915->drm,
> +		drm_dbg(&uncore->i915->drm,
>  			"warning: pcode (write of 0x%08x to mbox %x) mailbox access failed for %ps: %d\n",
>  			val, mbox, __builtin_return_address(0), err);
>  	}
> @@ -130,18 +128,18 @@ int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
>  	return err;
>  }
>  
> -static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> +static bool skl_pcode_try_request(struct intel_uncore *uncore, u32 mbox,
>  				  u32 request, u32 reply_mask, u32 reply,
>  				  u32 *status)
>  {
> -	*status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
> +	*status = __snb_pcode_rw(uncore, mbox, &request, NULL, 500, 0, true);
>  
>  	return (*status == 0) && ((request & reply_mask) == reply);
>  }
>  
>  /**
>   * skl_pcode_request - send PCODE request until acknowledgment
> - * @i915: device private
> + * @uncore: uncore
>   * @mbox: PCODE mailbox ID the request is targeted for
>   * @request: request ID
>   * @reply_mask: mask used to check for request acknowledgment
> @@ -158,16 +156,16 @@ static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
>   * Returns 0 on success, %-ETIMEDOUT in case of a timeout, <0 in case of some
>   * other error as reported by PCODE.
>   */
> -int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> +int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
>  		      u32 reply_mask, u32 reply, int timeout_base_ms)
>  {
>  	u32 status;
>  	int ret;
>  
> -	mutex_lock(&i915->sb_lock);
> +	mutex_lock(&uncore->i915->sb_lock);
>  
>  #define COND \
> -	skl_pcode_try_request(i915, mbox, request, reply_mask, reply, &status)
> +	skl_pcode_try_request(uncore, mbox, request, reply_mask, reply, &status)
>  
>  	/*
>  	 * Prime the PCODE by doing a request first. Normally it guarantees
> @@ -193,35 +191,26 @@ int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
>  	 * requests, and for any quirks of the PCODE firmware that delays
>  	 * the request completion.
>  	 */
> -	drm_dbg_kms(&i915->drm,
> +	drm_dbg_kms(&uncore->i915->drm,
>  		    "PCODE timeout, retrying with preemption disabled\n");
> -	drm_WARN_ON_ONCE(&i915->drm, timeout_base_ms > 3);
> +	drm_WARN_ON_ONCE(&uncore->i915->drm, timeout_base_ms > 3);
>  	preempt_disable();
>  	ret = wait_for_atomic(COND, 50);
>  	preempt_enable();
>  
>  out:
> -	mutex_unlock(&i915->sb_lock);
> +	mutex_unlock(&uncore->i915->sb_lock);
>  	return status ? status : ret;
>  #undef COND
>  }
>  
> -int intel_pcode_init(struct drm_i915_private *i915)
> +int intel_pcode_init(struct intel_uncore *uncore)
>  {
> -	int ret = 0;
> -
> -	if (!IS_DGFX(i915))
> -		return ret;
> -
> -	ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
> -				DG1_UNCORE_GET_INIT_STATUS,
> -				DG1_UNCORE_INIT_STATUS_COMPLETE,
> -				DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
> -
> -	drm_dbg(&i915->drm, "PCODE init status %d\n", ret);
> -
> -	if (ret)
> -		drm_err(&i915->drm, "Pcode did not report uncore initialization completion!\n");
> +	if (!IS_DGFX(uncore->i915))
> +		return 0;
>  
> -	return ret;
> +	return skl_pcode_request(uncore, DG1_PCODE_STATUS,
> +				 DG1_UNCORE_GET_INIT_STATUS,
> +				 DG1_UNCORE_INIT_STATUS_COMPLETE,
> +				 DG1_UNCORE_INIT_STATUS_COMPLETE, 180000);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_pcode.h b/drivers/gpu/drm/i915/intel_pcode.h
> index 0962a17fac48..8f6241b114a5 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.h
> +++ b/drivers/gpu/drm/i915/intel_pcode.h
> @@ -8,17 +8,17 @@
>  
>  #include <linux/types.h>
>  
> -struct drm_i915_private;
> +struct intel_uncore;
>  
> -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 *val1);
> -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
> +int snb_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 *val1);
> +int snb_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 val,
>  			    int fast_timeout_us, int slow_timeout_ms);
> -#define snb_pcode_write(i915, mbox, val)			\
> -	snb_pcode_write_timeout(i915, mbox, val, 500, 0)
> +#define snb_pcode_write(uncore, mbox, val) \
> +	snb_pcode_write_timeout(uncore, mbox, val, 500, 0)
>  
> -int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> +int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request,
>  		      u32 reply_mask, u32 reply, int timeout_base_ms);
>  
> -int intel_pcode_init(struct drm_i915_private *i915);
> +int intel_pcode_init(struct intel_uncore *uncore);
>  
>  #endif /* _INTEL_PCODE_H */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ee0047fdc95d..aacb21cbc62e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2874,7 +2874,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
>  
>  		/* read the first set of memory latencies[0:3] */
>  		val = 0; /* data0 to be programmed to 0 for first set */
> -		ret = snb_pcode_read(dev_priv, GEN9_PCODE_READ_MEM_LATENCY,
> +		ret = snb_pcode_read(&dev_priv->uncore, GEN9_PCODE_READ_MEM_LATENCY,
>  				     &val, NULL);
>  
>  		if (ret) {
> @@ -2893,7 +2893,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
>  
>  		/* read the second set of memory latencies[4:7] */
>  		val = 1; /* data0 to be programmed to 1 for second set */
> -		ret = snb_pcode_read(dev_priv, GEN9_PCODE_READ_MEM_LATENCY,
> +		ret = snb_pcode_read(&dev_priv->uncore, GEN9_PCODE_READ_MEM_LATENCY,
>  				     &val, NULL);
>  		if (ret) {
>  			drm_err(&dev_priv->drm,
> @@ -3679,7 +3679,7 @@ intel_sagv_block_time(struct drm_i915_private *dev_priv)
>  		u32 val = 0;
>  		int ret;
>  
> -		ret = snb_pcode_read(dev_priv,
> +		ret = snb_pcode_read(&dev_priv->uncore,
>  				     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
>  				     &val, NULL);
>  		if (ret) {
> @@ -3748,7 +3748,7 @@ static void skl_sagv_enable(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	drm_dbg_kms(&dev_priv->drm, "Enabling SAGV\n");
> -	ret = snb_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,
> +	ret = snb_pcode_write(&dev_priv->uncore, GEN9_PCODE_SAGV_CONTROL,
>  			      GEN9_SAGV_ENABLE);
>  
>  	/* We don't need to wait for SAGV when enabling */
> @@ -3781,7 +3781,7 @@ static void skl_sagv_disable(struct drm_i915_private *dev_priv)
>  
>  	drm_dbg_kms(&dev_priv->drm, "Disabling SAGV\n");
>  	/* bspec says to keep retrying for at least 1 ms */
> -	ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL,
> +	ret = skl_pcode_request(&dev_priv->uncore, GEN9_PCODE_SAGV_CONTROL,
>  				GEN9_SAGV_DISABLE,
>  				GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
>  				1);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 3/7] drm/i915/pcode: Extend pcode functions for multiple gt's

[Dixit, Ashutosh]

On Thu, 12 May 2022 03:36:31 -0700, Jani Nikula wrote:
>

Hi Jani,

> On Wed, 11 May 2022, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> > Each gt contains an independent instance of pcode. Extend pcode functions
> > to interface with pcode on different gt's. To avoid creating dependency of
> > display functionality on intel_gt, pcode function interfaces are exposed in
> > terms of uncore rather than intel_gt. Callers have been converted to pass
> > in the appropritate (i915 or intel_gt) uncore to the pcode functions.
> >
> > v2: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
> > v3: Retain previous function names to eliminate needless #defines (Rodrigo)
> > v4: Move out i915_pcode_init() to a separate patch (Tvrtko)
> >     Remove duplicated drm_err/drm_dbg from intel_pcode_init() (Tvrtko)
>
> Couple of nitpicks inline, and not insisting on changing. Basically ack
> on this from me.

Thanks!

> > 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 0c6b9eb724ae..90a440865037 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> > @@ -138,7 +138,7 @@ static int gen6_drpc(struct seq_file *m)
> >	}
> >
> >	if (GRAPHICS_VER(i915) <= 7)
> > -		snb_pcode_read(i915, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
> > +		snb_pcode_read(gt->uncore, GEN6_PCODE_READ_RC6VIDS, &rc6vids, NULL);
>
> Pedantically, I'm wondering if this (and similar places) should first be
> i915->uncore, to be replaced with gt->uncore in the next patch.

I did think about this and what you are suggesting is definitely possible
since we have an i915 variable. But on the other hand these structures are
already inside a gt and so 'i915->uncore' is really a 'gt->i915->uncore' so
someone might say why don't you just do a 'gt->uncore' which is the same as
'gt->i915->uncore'. But we could do it over two patches as you suggest.

> > diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c
> > index ac727546868e..2be700932322 100644
> > --- a/drivers/gpu/drm/i915/intel_pcode.c
> > +++ b/drivers/gpu/drm/i915/intel_pcode.c
> > @@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox)
> >	}
> >  }
> >
> > -static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
> > +static int __snb_pcode_rw(struct intel_uncore *uncore, u32 mbox,
> >			  u32 *val, u32 *val1,
> >			  int fast_timeout_us, int slow_timeout_ms,
> >			  bool is_read)
> >  {
> > -	struct intel_uncore *uncore = &i915->uncore;
>
> Nitpick, personally, I would probably have just replaced the above with
>
>	struct drm_i915_private *i915 = uncore->i915;
>
> to minimize the diff. Ditto everywhere. But not a big deal.

Agreed.

Since you seem to be ok I am tending to not spin these patches again. But
if you feel otherwise please let me know and I can do it.

Thanks.
--
Ashutosh

Re: [Intel-gfx] [PATCH v5 0/7] drm/i915: Media freq factor and per-gt enhancements/fixes

[Dixit, Ashutosh]

On Thu, 12 May 2022 00:59:11 -0700, Tvrtko Ursulin wrote:

Hi Tvrtko,

> On 12/05/2022 05:38, Dixit, Ashutosh wrote:
> > On Wed, 11 May 2022 19:32:13 -0700, Ashutosh Dixit wrote:
> >>
> >> Some recent Intel dGfx platforms allow media IP to work at a different
> >> frequency from the base GT. This patch series exposes sysfs controls for
> >> this functionality in the new per-gt sysfs. Some enhancements and fixes to
> >> previous per-gt functionality are also included to complete the new
> >> functionality:
> >> * Patches 1 and 2 implement basic sysfs controls for media freq
> >> * Patch 3 extends previous pcode functions for multiple gt's
> >> * Patch 4 inits pcode on different gt's
> >> * Patch 5 adds a couple of pcode helpers
> >> * Patch 6 uses the new pcode functions to retrieve media RP0/RPn freq
> >> * Patch 7 fixes memory leaks in the previous per-gt sysfs implementation
> >>    and some code refactoring
> >
> > In this v5 I have dropped the last two patches of the v4 series, these
> > ones:
> >
> > [PATCH 7/8] drm/i915/gt: Expose per-gt RPS defaults in sysfs
> > [PATCH 8/8] drm/i915/gt: Expose default value for media_freq_factor in per-gt sysfs
> >
> > Because these need more work based on the review comments. If this series
> > is merged I will submit these patches as a separate series, otherwise I
> > will re-add them to this series and resubmit (due to dependence between
> > this series and those patches).
> >
> > Apart from this, I believe I have addressed all previous review comments on
> > the patches in this series.
> >
> > Thanks for reviewing,
>
> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> At some point we should starting documenting our sysfs in
> Documentation/ABI. For instance the freq ratio this patch adds really
> does need some user facing docs to know how to use it. Would you sign up
> to document the bits this series adds as follow up?

Yes, sure. Looks like none of the previous i915 sysfs (even the per-device
one) is in Documentation/ABI so we'll need to figure out how we are doing
to organize the files, e.g. just one big
Documentation/ABI/testing/sysfs-driver-intel-i915 file or multiple files
organized somehow. So let's do this as follow up.

Thanks.
--
Ashutosh

Re: [Intel-gfx] [PATCH 4/7] drm/i915/pcode: Init pcode on different gt's

[Tvrtko Ursulin]

On 13/05/2022 02:36, Ashutosh Dixit wrote:
> Extend pcode initialization to pcode on different gt's.
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_driver.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 0e9763868d68..e137bcf021ee 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -520,6 +520,22 @@ static int i915_set_dma_info(struct drm_i915_private *i915)
>   	return ret;
>   }
>   
> +static int i915_pcode_init(struct drm_i915_private *i915)
> +{
> +	struct intel_gt *gt;
> +	int id, ret;
> +
> +	for_each_gt(gt, i915, id) {
> +		ret = intel_pcode_init(gt->uncore);
> +		if (ret) {
> +			drm_err(&gt->i915->drm, "gt%d: intel_pcode_init failed %d\n", id, ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * i915_driver_hw_probe - setup state requiring device access
>    * @dev_priv: device private
> @@ -629,7 +645,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>   
>   	intel_opregion_setup(dev_priv);
>   
> -	ret = intel_pcode_init(&dev_priv->uncore);
> +	ret = i915_pcode_init(dev_priv);
>   	if (ret)
>   		goto err_msi;
>   
> @@ -1251,7 +1267,7 @@ static int i915_drm_resume(struct drm_device *dev)
>   
>   	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>   
> -	ret = intel_pcode_init(&dev_priv->uncore);
> +	ret = i915_pcode_init(dev_priv);
>   	if (ret)
>   		return ret;
>   

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

[Intel-gfx] [PATCH 4/7] drm/i915/pcode: Init pcode on different gt's

[Ashutosh Dixit]

Extend pcode initialization to pcode on different gt's.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 0e9763868d68..e137bcf021ee 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -520,6 +520,22 @@ static int i915_set_dma_info(struct drm_i915_private *i915)
 	return ret;
 }
 
+static int i915_pcode_init(struct drm_i915_private *i915)
+{
+	struct intel_gt *gt;
+	int id, ret;
+
+	for_each_gt(gt, i915, id) {
+		ret = intel_pcode_init(gt->uncore);
+		if (ret) {
+			drm_err(&gt->i915->drm, "gt%d: intel_pcode_init failed %d\n", id, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * i915_driver_hw_probe - setup state requiring device access
  * @dev_priv: device private
@@ -629,7 +645,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
 
 	intel_opregion_setup(dev_priv);
 
-	ret = intel_pcode_init(&dev_priv->uncore);
+	ret = i915_pcode_init(dev_priv);
 	if (ret)
 		goto err_msi;
 
@@ -1251,7 +1267,7 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
-	ret = intel_pcode_init(&dev_priv->uncore);
+	ret = i915_pcode_init(dev_priv);
 	if (ret)
 		return ret;
 
-- 
2.34.1