[PATCH 0/4] drm/i915: PMINTRMSK fix for VLV/CHV and some other rps stuff

[PATCH 0/4] drm/i915: PMINTRMSK fix for VLV/CHV and some other rps stuff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I was staring at the GPU frequency on my BSW recently when I noticed that
the value of PMINTRMSK didn't make much sense when I was frobbing with the
sysfs frequency files. So while fixing that I became annoyed at the
differences between VLV/CHV vs. others and tried to do something about it.

Tested on BSW and IVB, and all the values still look OK, so I'm going
to optimistically say that I didn't mess up anything with the semantic
patch.

Ville Syrjälä (4):
  drm/i915: Update PMINTRMSK on VLV/CHV after sysfs min/max freq change
  drm/i915: Add gt_act_freq_mhz sysfs file
  drm/i915: Add intel_gpu_freq() and intel_freq_opcode()
  drm/i915: Use intel_gpu_freq() and intel_freq_opcode()

 drivers/gpu/drm/i915/i915_debugfs.c |  43 ++++++-------
 drivers/gpu/drm/i915/i915_drv.h     |   4 +-
 drivers/gpu/drm/i915/i915_sysfs.c   | 121 +++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_pm.c     |  50 +++++++--------
 4 files changed, 115 insertions(+), 103 deletions(-)

-- 
2.0.5

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

[PATCH 1/4] drm/i915: Update PMINTRMSK on VLV/CHV after sysfs min/max freq change

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we don't call valleyview_set_rps() when changing the min/max
limits through sysfs if the current frequency is still within the new
limits. However that means we sometimes forget to update PMINTRMSK.
Eg. if the current frequency is at the old minimum, and then we reduce
the minum further we should then enable the 'down' interrupts in PMINTRMSK
but currently we don't.

Fix it up by always calling valleyview_set_rps() (just like we do for
!vlv/chv platforms). This also allows the code to be simplified a bit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 44 +++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index db14d3e..422b563 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -393,17 +393,17 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	dev_priv->rps.max_freq_softlimit = val;
 
-	if (dev_priv->rps.cur_freq > val) {
-		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev, val);
-		else
-			gen6_set_rps(dev, val);
-	} else if (!IS_VALLEYVIEW(dev)) {
-		/* We still need gen6_set_rps to process the new max_delay and
-		 * update the interrupt limits even though frequency request is
-		 * unchanged. */
-		gen6_set_rps(dev, dev_priv->rps.cur_freq);
-	}
+	val = clamp_t(int, dev_priv->rps.cur_freq,
+		      dev_priv->rps.min_freq_softlimit,
+		      dev_priv->rps.max_freq_softlimit);
+
+	/* We still need *_set_rps to process the new max_delay and
+	 * update the interrupt limits and PMINTRMSK even though
+	 * frequency request may be unchanged. */
+	if (IS_VALLEYVIEW(dev))
+		valleyview_set_rps(dev, val);
+	else
+		gen6_set_rps(dev, val);
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
@@ -461,17 +461,17 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	dev_priv->rps.min_freq_softlimit = val;
 
-	if (dev_priv->rps.cur_freq < val) {
-		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev, val);
-		else
-			gen6_set_rps(dev, val);
-	} else if (!IS_VALLEYVIEW(dev)) {
-		/* We still need gen6_set_rps to process the new min_delay and
-		 * update the interrupt limits even though frequency request is
-		 * unchanged. */
-		gen6_set_rps(dev, dev_priv->rps.cur_freq);
-	}
+	val = clamp_t(int, dev_priv->rps.cur_freq,
+		      dev_priv->rps.min_freq_softlimit,
+		      dev_priv->rps.max_freq_softlimit);
+
+	/* We still need *_set_rps to process the new min_delay and
+	 * update the interrupt limits and PMINTRMSK even though
+	 * frequency request may be unchanged. */
+	if (IS_VALLEYVIEW(dev))
+		valleyview_set_rps(dev, val);
+	else
+		gen6_set_rps(dev, val);
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-- 
2.0.5

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

[PATCH 2/4] drm/i915: Add gt_act_freq_mhz sysfs file

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently the 'gt_cur_freq_mhz' file shows the actual GPU frequency on
VLV/CHV, and the last requested frequency on other platforms. Change the
meaning of the file on VLV/CHV to follow the the other platforms, and
introduce a new file 'gt_act_freq_mhz' which shows the actual frequency
on all platforms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 422b563..f99543b 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -300,7 +300,7 @@ static struct bin_attribute dpf_attrs_1 = {
 	.private = (void *)1
 };
 
-static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
+static ssize_t gt_act_freq_mhz_show(struct device *kdev,
 				    struct device_attribute *attr, char *buf)
 {
 	struct drm_minor *minor = dev_to_drm_minor(kdev);
@@ -318,6 +318,36 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 		ret = vlv_gpu_freq(dev_priv, (freq >> 8) & 0xff);
 	} else {
+		u32 rpstat = I915_READ(GEN6_RPSTAT1);
+		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+			ret = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
+		else
+			ret = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
+		ret *= GT_FREQUENCY_MULTIPLIER;
+	}
+	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	intel_runtime_pm_put(dev_priv);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
+}
+
+static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = dev_to_drm_minor(kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
+
+	intel_runtime_pm_get(dev_priv);
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	if (IS_VALLEYVIEW(dev_priv->dev)) {
+		ret = vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
+	} else {
 		ret = dev_priv->rps.cur_freq * GT_FREQUENCY_MULTIPLIER;
 	}
 	mutex_unlock(&dev_priv->rps.hw_lock);
@@ -479,6 +509,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 }
 
+static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show, NULL);
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
 static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
 static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
@@ -529,6 +560,7 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
 }
 
 static const struct attribute *gen6_attrs[] = {
+	&dev_attr_gt_act_freq_mhz.attr,
 	&dev_attr_gt_cur_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
@@ -539,6 +571,7 @@ static const struct attribute *gen6_attrs[] = {
 };
 
 static const struct attribute *vlv_attrs[] = {
+	&dev_attr_gt_act_freq_mhz.attr,
 	&dev_attr_gt_cur_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
-- 
2.0.5

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

[PATCH 3/4] drm/i915: Add intel_gpu_freq() and intel_freq_opcode()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rename the vlv_gpu_freq() and vlv_freq_opecode() functions to have
an intel_ prefix, and handle non-VLV/CHV platforms in them as well.
Leave the vlv_ names around for now since they're currently used.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c | 30 ++++++++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0d67b17..102569e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3233,6 +3233,8 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 
+int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
+int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
 int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e630fe..bc243a2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6615,28 +6615,34 @@ static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
 	return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2;
 }
 
-int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
+int intel_gpu_freq(struct drm_i915_private *dev_priv, int val)
 {
-	int ret = -1;
-
 	if (IS_CHERRYVIEW(dev_priv->dev))
-		ret = chv_gpu_freq(dev_priv, val);
+		return chv_gpu_freq(dev_priv, val);
 	else if (IS_VALLEYVIEW(dev_priv->dev))
-		ret = byt_gpu_freq(dev_priv, val);
-
-	return ret;
+		return byt_gpu_freq(dev_priv, val);
+	else
+		return val * GT_FREQUENCY_MULTIPLIER;
 }
 
-int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
+int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
 {
-	int ret = -1;
+	return intel_gpu_freq(dev_priv, val);
+}
 
+int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
+{
 	if (IS_CHERRYVIEW(dev_priv->dev))
-		ret = chv_freq_opcode(dev_priv, val);
+		return chv_freq_opcode(dev_priv, val);
 	else if (IS_VALLEYVIEW(dev_priv->dev))
-		ret = byt_freq_opcode(dev_priv, val);
+		return byt_freq_opcode(dev_priv, val);
+	else
+		return val / GT_FREQUENCY_MULTIPLIER;
+}
 
-	return ret;
+int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
+{
+	return intel_freq_opcode(dev_priv, val);
 }
 
 void intel_pm_setup(struct drm_device *dev)
-- 
2.0.5

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

[PATCH 4/4] drm/i915: Use intel_gpu_freq() and intel_freq_opcode()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace all the vlv_gpu_freq(), vlv_freq_opcode(),
*GT_FREQUENCY_MULTIPLIER, and /GT_FREQUENCY_MULTIPLIER instances
with intel_gpu_freq() and intel_freq_opcode() calls.

Most of the change was performed with the following semantic patch:
@@
expression E;
@@
(
- E * GT_FREQUENCY_MULTIPLIER
+ intel_gpu_freq(dev_priv, E)
|
- E *= GT_FREQUENCY_MULTIPLIER
+ E = intel_gpu_freq(dev_priv, E)
|
- E /= GT_FREQUENCY_MULTIPLIER
+ E = intel_freq_opcode(dev_priv, E)
|
- do_div(E, GT_FREQUENCY_MULTIPLIER)
+ E = intel_freq_opcode(dev_priv, E)
)

@@
expression E1, E2;
@@
(
- vlv_gpu_freq(E1, E2)
+ intel_gpu_freq(E1, E2)
|
- vlv_freq_opcode(E1, E2)
+ intel_freq_opcode(E1, E2)
)

@@
expression E1, E2, E3, E4;
@@
(
- if (IS_VALLEYVIEW(E1)) {
-  E2 = intel_gpu_freq(E3, E4);
- } else {
-  E2 = intel_gpu_freq(E3, E4);
- }
+ E2 = intel_gpu_freq(E3, E4);
|
- if (IS_VALLEYVIEW(E1)) {
-  E2 = intel_freq_opcode(E3, E4);
- } else {
-  E2 = intel_freq_opcode(E3, E4);
- }
+ E2 = intel_freq_opcode(E3, E4);
)

One hunk was manually undone as intel_gpu_freq() ended up
calling itself. Supposedly it would be possible to exclude
certain functions via !=~, but I couldn't get that to work.

Also the removal of vlv_gpu_freq() and vlv_opcode_freq() compat
wrappers was done manually.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 43 ++++++++++++++----------------
 drivers/gpu/drm/i915/i915_drv.h     |  2 --
 drivers/gpu/drm/i915/i915_sysfs.c   | 52 ++++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_pm.c     | 36 ++++++++++---------------
 4 files changed, 52 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ad4c48..b315f01 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1113,7 +1113,7 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 			reqf >>= 24;
 		else
 			reqf >>= 25;
-		reqf *= GT_FREQUENCY_MULTIPLIER;
+		reqf = intel_gpu_freq(dev_priv, reqf);
 
 		rpmodectl = I915_READ(GEN6_RP_CONTROL);
 		rpinclimit = I915_READ(GEN6_RP_UP_THRESHOLD);
@@ -1130,7 +1130,7 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 			cagf = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
 		else
 			cagf = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
-		cagf *= GT_FREQUENCY_MULTIPLIER;
+		cagf = intel_gpu_freq(dev_priv, cagf);
 
 		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 		mutex_unlock(&dev->struct_mutex);
@@ -1178,18 +1178,18 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 
 		max_freq = (rp_state_cap & 0xff0000) >> 16;
 		seq_printf(m, "Lowest (RPN) frequency: %dMHz\n",
-			   max_freq * GT_FREQUENCY_MULTIPLIER);
+			   intel_gpu_freq(dev_priv, max_freq));
 
 		max_freq = (rp_state_cap & 0xff00) >> 8;
 		seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
-			   max_freq * GT_FREQUENCY_MULTIPLIER);
+			   intel_gpu_freq(dev_priv, max_freq));
 
 		max_freq = rp_state_cap & 0xff;
 		seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
-			   max_freq * GT_FREQUENCY_MULTIPLIER);
+			   intel_gpu_freq(dev_priv, max_freq));
 
 		seq_printf(m, "Max overclocked frequency: %dMHz\n",
-			   dev_priv->rps.max_freq * GT_FREQUENCY_MULTIPLIER);
+			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
 	} else if (IS_VALLEYVIEW(dev)) {
 		u32 freq_sts;
 
@@ -1199,16 +1199,17 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 		seq_printf(m, "DDR freq: %d MHz\n", dev_priv->mem_freq);
 
 		seq_printf(m, "max GPU freq: %d MHz\n",
-			   vlv_gpu_freq(dev_priv, dev_priv->rps.max_freq));
+			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
 
 		seq_printf(m, "min GPU freq: %d MHz\n",
-			   vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq));
+			   intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
 
-		seq_printf(m, "efficient (RPe) frequency: %d MHz\n",
-			   vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
+		seq_printf(m,
+			   "efficient (RPe) frequency: %d MHz\n",
+			   intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
 
 		seq_printf(m, "current GPU freq: %d MHz\n",
-			   vlv_gpu_freq(dev_priv, (freq_sts >> 8) & 0xff));
+			   intel_gpu_freq(dev_priv, (freq_sts >> 8) & 0xff));
 		mutex_unlock(&dev_priv->rps.hw_lock);
 	} else {
 		seq_puts(m, "no P-state info available\n");
@@ -1677,7 +1678,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 				       GEN6_PCODE_READ_MIN_FREQ_TABLE,
 				       &ia_freq);
 		seq_printf(m, "%d\t\t%d\t\t\t\t%d\n",
-			   gpu_freq * GT_FREQUENCY_MULTIPLIER,
+			   intel_gpu_freq(dev_priv, gpu_freq),
 			   ((ia_freq >> 0) & 0xff) * 100,
 			   ((ia_freq >> 8) & 0xff) * 100);
 	}
@@ -4119,10 +4120,7 @@ i915_max_freq_get(void *data, u64 *val)
 	if (ret)
 		return ret;
 
-	if (IS_VALLEYVIEW(dev))
-		*val = vlv_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
-	else
-		*val = dev_priv->rps.max_freq_softlimit * GT_FREQUENCY_MULTIPLIER;
+	*val = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return 0;
@@ -4151,12 +4149,12 @@ i915_max_freq_set(void *data, u64 val)
 	 * Turbo will still be enabled, but won't go above the set value.
 	 */
 	if (IS_VALLEYVIEW(dev)) {
-		val = vlv_freq_opcode(dev_priv, val);
+		val = intel_freq_opcode(dev_priv, val);
 
 		hw_max = dev_priv->rps.max_freq;
 		hw_min = dev_priv->rps.min_freq;
 	} else {
-		do_div(val, GT_FREQUENCY_MULTIPLIER);
+		val = intel_freq_opcode(dev_priv, val);
 
 		rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
 		hw_max = dev_priv->rps.max_freq;
@@ -4200,10 +4198,7 @@ i915_min_freq_get(void *data, u64 *val)
 	if (ret)
 		return ret;
 
-	if (IS_VALLEYVIEW(dev))
-		*val = vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
-	else
-		*val = dev_priv->rps.min_freq_softlimit * GT_FREQUENCY_MULTIPLIER;
+	*val = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return 0;
@@ -4232,12 +4227,12 @@ i915_min_freq_set(void *data, u64 val)
 	 * Turbo will still be enabled, but won't go below the set value.
 	 */
 	if (IS_VALLEYVIEW(dev)) {
-		val = vlv_freq_opcode(dev_priv, val);
+		val = intel_freq_opcode(dev_priv, val);
 
 		hw_max = dev_priv->rps.max_freq;
 		hw_min = dev_priv->rps.min_freq;
 	} else {
-		do_div(val, GT_FREQUENCY_MULTIPLIER);
+		val = intel_freq_opcode(dev_priv, val);
 
 		rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
 		hw_max = dev_priv->rps.max_freq;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 102569e..e725fb4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3235,8 +3235,6 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 
 int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
-int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val);
-int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
 
 #define I915_READ8(reg)		dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
 #define I915_WRITE8(reg, val)	dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index f99543b..4693aae 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -316,14 +316,14 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev,
 	if (IS_VALLEYVIEW(dev_priv->dev)) {
 		u32 freq;
 		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
-		ret = vlv_gpu_freq(dev_priv, (freq >> 8) & 0xff);
+		ret = intel_gpu_freq(dev_priv, (freq >> 8) & 0xff);
 	} else {
 		u32 rpstat = I915_READ(GEN6_RPSTAT1);
 		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 			ret = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
 		else
 			ret = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
-		ret *= GT_FREQUENCY_MULTIPLIER;
+		ret = intel_gpu_freq(dev_priv, ret);
 	}
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
@@ -345,11 +345,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 	intel_runtime_pm_get(dev_priv);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
-	if (IS_VALLEYVIEW(dev_priv->dev)) {
-		ret = vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
-	} else {
-		ret = dev_priv->rps.cur_freq * GT_FREQUENCY_MULTIPLIER;
-	}
+	ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	intel_runtime_pm_put(dev_priv);
@@ -364,8 +360,9 @@ static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	return snprintf(buf, PAGE_SIZE, "%d\n",
-			vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
+	return snprintf(buf, PAGE_SIZE,
+			"%d\n",
+			intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
 }
 
 static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
@@ -378,10 +375,7 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
-	if (IS_VALLEYVIEW(dev_priv->dev))
-		ret = vlv_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
-	else
-		ret = dev_priv->rps.max_freq_softlimit * GT_FREQUENCY_MULTIPLIER;
+	ret = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
@@ -405,10 +399,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	mutex_lock(&dev_priv->rps.hw_lock);
 
-	if (IS_VALLEYVIEW(dev_priv->dev))
-		val = vlv_freq_opcode(dev_priv, val);
-	else
-		val /= GT_FREQUENCY_MULTIPLIER;
+	val = intel_freq_opcode(dev_priv, val);
 
 	if (val < dev_priv->rps.min_freq ||
 	    val > dev_priv->rps.max_freq ||
@@ -419,7 +410,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	if (val > dev_priv->rps.rp0_freq)
 		DRM_DEBUG("User requested overclocking to %d\n",
-			  val * GT_FREQUENCY_MULTIPLIER);
+			  intel_gpu_freq(dev_priv, val));
 
 	dev_priv->rps.max_freq_softlimit = val;
 
@@ -450,10 +441,7 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
-	if (IS_VALLEYVIEW(dev_priv->dev))
-		ret = vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
-	else
-		ret = dev_priv->rps.min_freq_softlimit * GT_FREQUENCY_MULTIPLIER;
+	ret = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
@@ -477,10 +465,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	mutex_lock(&dev_priv->rps.hw_lock);
 
-	if (IS_VALLEYVIEW(dev))
-		val = vlv_freq_opcode(dev_priv, val);
-	else
-		val /= GT_FREQUENCY_MULTIPLIER;
+	val = intel_freq_opcode(dev_priv, val);
 
 	if (val < dev_priv->rps.min_freq ||
 	    val > dev_priv->rps.max_freq ||
@@ -540,19 +525,22 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
 
 	if (attr == &dev_attr_gt_RP0_freq_mhz) {
 		if (IS_VALLEYVIEW(dev))
-			val = vlv_gpu_freq(dev_priv, dev_priv->rps.rp0_freq);
+			val = intel_gpu_freq(dev_priv, dev_priv->rps.rp0_freq);
 		else
-			val = ((rp_state_cap & 0x0000ff) >> 0) * GT_FREQUENCY_MULTIPLIER;
+			val = intel_gpu_freq(dev_priv,
+					     ((rp_state_cap & 0x0000ff) >> 0));
 	} else if (attr == &dev_attr_gt_RP1_freq_mhz) {
 		if (IS_VALLEYVIEW(dev))
-			val = vlv_gpu_freq(dev_priv, dev_priv->rps.rp1_freq);
+			val = intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq);
 		else
-			val = ((rp_state_cap & 0x00ff00) >> 8) * GT_FREQUENCY_MULTIPLIER;
+			val = intel_gpu_freq(dev_priv,
+					     ((rp_state_cap & 0x00ff00) >> 8));
 	} else if (attr == &dev_attr_gt_RPn_freq_mhz) {
 		if (IS_VALLEYVIEW(dev))
-			val = vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq);
+			val = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq);
 		else
-			val = ((rp_state_cap & 0xff0000) >> 16) * GT_FREQUENCY_MULTIPLIER;
+			val = intel_gpu_freq(dev_priv,
+					     ((rp_state_cap & 0xff0000) >> 16));
 	} else {
 		BUG();
 	}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bc243a2..dbeecb0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3883,7 +3883,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	dev_priv->rps.cur_freq = val;
-	trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv, val));
+	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
 
 static void gen9_disable_rps(struct drm_device *dev)
@@ -4619,22 +4619,22 @@ static void valleyview_init_gt_powersave(struct drm_device *dev)
 	dev_priv->rps.max_freq = valleyview_rps_max_freq(dev_priv);
 	dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
 	DRM_DEBUG_DRIVER("max GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.max_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.max_freq),
 			 dev_priv->rps.max_freq);
 
 	dev_priv->rps.efficient_freq = valleyview_rps_rpe_freq(dev_priv);
 	DRM_DEBUG_DRIVER("RPe GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
 			 dev_priv->rps.efficient_freq);
 
 	dev_priv->rps.rp1_freq = valleyview_rps_guar_freq(dev_priv);
 	DRM_DEBUG_DRIVER("RP1(Guar Freq) GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
 	dev_priv->rps.min_freq = valleyview_rps_min_freq(dev_priv);
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);
 
 	/* Preserve min/max settings in case of re-init */
@@ -4688,22 +4688,22 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 	dev_priv->rps.max_freq = cherryview_rps_max_freq(dev_priv);
 	dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
 	DRM_DEBUG_DRIVER("max GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.max_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.max_freq),
 			 dev_priv->rps.max_freq);
 
 	dev_priv->rps.efficient_freq = cherryview_rps_rpe_freq(dev_priv);
 	DRM_DEBUG_DRIVER("RPe GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
 			 dev_priv->rps.efficient_freq);
 
 	dev_priv->rps.rp1_freq = cherryview_rps_guar_freq(dev_priv);
 	DRM_DEBUG_DRIVER("RP1(Guar) GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
 	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);
 
 	WARN_ONCE((dev_priv->rps.max_freq |
@@ -4807,11 +4807,11 @@ static void cherryview_enable_rps(struct drm_device *dev)
 
 	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
 	DRM_DEBUG_DRIVER("current GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
 			 dev_priv->rps.cur_freq);
 
 	DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
 			 dev_priv->rps.efficient_freq);
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
@@ -4891,11 +4891,11 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
 	DRM_DEBUG_DRIVER("current GPU freq: %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
 			 dev_priv->rps.cur_freq);
 
 	DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n",
-			 vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
+			 intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
 			 dev_priv->rps.efficient_freq);
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
@@ -6625,11 +6625,6 @@ int intel_gpu_freq(struct drm_i915_private *dev_priv, int val)
 		return val * GT_FREQUENCY_MULTIPLIER;
 }
 
-int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
-{
-	return intel_gpu_freq(dev_priv, val);
-}
-
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
 {
 	if (IS_CHERRYVIEW(dev_priv->dev))
@@ -6640,11 +6635,6 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
 		return val / GT_FREQUENCY_MULTIPLIER;
 }
 
-int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
-{
-	return intel_freq_opcode(dev_priv, val);
-}
-
 void intel_pm_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
2.0.5

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

Re: [PATCH 1/4] drm/i915: Update PMINTRMSK on VLV/CHV after sysfs min/max freq change

[Chris Wilson]

On Fri, Jan 23, 2015 at 09:04:23PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we don't call valleyview_set_rps() when changing the min/max
> limits through sysfs if the current frequency is still within the new
> limits. However that means we sometimes forget to update PMINTRMSK.
> Eg. if the current frequency is at the old minimum, and then we reduce
> the minum further we should then enable the 'down' interrupts in PMINTRMSK
> but currently we don't.
> 
> Fix it up by always calling valleyview_set_rps() (just like we do for
> !vlv/chv platforms). This also allows the code to be simplified a bit.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> +	/* We still need *_set_rps to process the new min_delay and
> +	 * update the interrupt limits and PMINTRMSK even though
> +	 * frequency request may be unchanged. */
> +	if (IS_VALLEYVIEW(dev))
> +		valleyview_set_rps(dev, val);
> +	else
> +		gen6_set_rps(dev, val);

Another peeve of mine is this split and all callers having to
choose which version to use. intel_rps_set_freq() please.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Update PMINTRMSK on VLV/CHV after sysfs min/max freq change

[Chris Wilson]

On Fri, Jan 23, 2015 at 09:04:23PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we don't call valleyview_set_rps() when changing the min/max
> limits through sysfs if the current frequency is still within the new
> limits. However that means we sometimes forget to update PMINTRMSK.
> Eg. if the current frequency is at the old minimum, and then we reduce
> the minum further we should then enable the 'down' interrupts in PMINTRMSK
> but currently we don't.
> 
> Fix it up by always calling valleyview_set_rps() (just like we do for
> !vlv/chv platforms). This also allows the code to be simplified a bit.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Add gt_act_freq_mhz sysfs file

[Chris Wilson]

On Fri, Jan 23, 2015 at 09:04:24PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently the 'gt_cur_freq_mhz' file shows the actual GPU frequency on
> VLV/CHV, and the last requested frequency on other platforms. Change the
> meaning of the file on VLV/CHV to follow the the other platforms, and
> introduce a new file 'gt_act_freq_mhz' which shows the actual frequency
> on all platforms.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

>  drivers/gpu/drm/i915/i915_sysfs.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct drm_minor *minor = dev_to_drm_minor(kdev);
> +	struct drm_device *dev = minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int ret;
> +
> +	flush_delayed_work(&dev_priv->rps.delayed_resume_work);

Is this required for querying the current value?

Though probably better to keep it similar to the others.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Add intel_gpu_freq() and intel_freq_opcode()

[Chris Wilson]

On Fri, Jan 23, 2015 at 09:04:25PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rename the vlv_gpu_freq() and vlv_freq_opecode() functions to have
> an intel_ prefix, and handle non-VLV/CHV platforms in them as well.
> Leave the vlv_ names around for now since they're currently used.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Use intel_gpu_freq() and intel_freq_opcode()

[Chris Wilson]

On Fri, Jan 23, 2015 at 09:04:26PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace all the vlv_gpu_freq(), vlv_freq_opcode(),
> *GT_FREQUENCY_MULTIPLIER, and /GT_FREQUENCY_MULTIPLIER instances
> with intel_gpu_freq() and intel_freq_opcode() calls.
> 
> Most of the change was performed with the following semantic patch:
> @@
> expression E;
> @@
> (
> - E * GT_FREQUENCY_MULTIPLIER
> + intel_gpu_freq(dev_priv, E)
> |
> - E *= GT_FREQUENCY_MULTIPLIER
> + E = intel_gpu_freq(dev_priv, E)
> |
> - E /= GT_FREQUENCY_MULTIPLIER
> + E = intel_freq_opcode(dev_priv, E)
> |
> - do_div(E, GT_FREQUENCY_MULTIPLIER)
> + E = intel_freq_opcode(dev_priv, E)
> )
> 
> @@
> expression E1, E2;
> @@
> (
> - vlv_gpu_freq(E1, E2)
> + intel_gpu_freq(E1, E2)
> |
> - vlv_freq_opcode(E1, E2)
> + intel_freq_opcode(E1, E2)
> )
> 
> @@
> expression E1, E2, E3, E4;
> @@
> (
> - if (IS_VALLEYVIEW(E1)) {
> -  E2 = intel_gpu_freq(E3, E4);
> - } else {
> -  E2 = intel_gpu_freq(E3, E4);
> - }
> + E2 = intel_gpu_freq(E3, E4);
> |
> - if (IS_VALLEYVIEW(E1)) {
> -  E2 = intel_freq_opcode(E3, E4);
> - } else {
> -  E2 = intel_freq_opcode(E3, E4);
> - }
> + E2 = intel_freq_opcode(E3, E4);
> )
> 
> One hunk was manually undone as intel_gpu_freq() ended up
> calling itself. Supposedly it would be possible to exclude
> certain functions via !=~, but I couldn't get that to work.
> 
> Also the removal of vlv_gpu_freq() and vlv_opcode_freq() compat
> wrappers was done manually.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Spot checked most,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Add gt_act_freq_mhz sysfs file

[O'Rourke, Tom]

On Sun, Jan 25, 2015 at 09:34:33AM +0000, Chris Wilson wrote:
> On Fri, Jan 23, 2015 at 09:04:24PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently the 'gt_cur_freq_mhz' file shows the actual GPU frequency on
> > VLV/CHV, and the last requested frequency on other platforms. Change the
> > meaning of the file on VLV/CHV to follow the the other platforms, and
> > introduce a new file 'gt_act_freq_mhz' which shows the actual frequency
> > on all platforms.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> >  drivers/gpu/drm/i915/i915_sysfs.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> > +				    struct device_attribute *attr, char *buf)
> > +{
> > +	struct drm_minor *minor = dev_to_drm_minor(kdev);
> > +	struct drm_device *dev = minor->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int ret;
> > +
> > +	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> 
> Is this required for querying the current value?
> 
> Though probably better to keep it similar to the others.
> -Chris

[TOR:] flush_delayed_work is needed to make sure rps.cur_freq
is valid for non VLV/CHV platforms.

Tom O'Rourke

> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Add gt_act_freq_mhz sysfs file

[Chris Wilson]

On Mon, Jan 26, 2015 at 12:22:07PM -0800, O'Rourke, Tom wrote:
> On Sun, Jan 25, 2015 at 09:34:33AM +0000, Chris Wilson wrote:
> > On Fri, Jan 23, 2015 at 09:04:24PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Currently the 'gt_cur_freq_mhz' file shows the actual GPU frequency on
> > > VLV/CHV, and the last requested frequency on other platforms. Change the
> > > meaning of the file on VLV/CHV to follow the the other platforms, and
> > > introduce a new file 'gt_act_freq_mhz' which shows the actual frequency
> > > on all platforms.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > >  drivers/gpu/drm/i915/i915_sysfs.c | 35 ++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 34 insertions(+), 1 deletion(-)
> > > 
> > > +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> > > +				    struct device_attribute *attr, char *buf)
> > > +{
> > > +	struct drm_minor *minor = dev_to_drm_minor(kdev);
> > > +	struct drm_device *dev = minor->dev;
> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +	int ret;
> > > +
> > > +	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> > 
> > Is this required for querying the current value?
> > 
> > Though probably better to keep it similar to the others.
> > -Chris
> 
> [TOR:] flush_delayed_work is needed to make sure rps.cur_freq
> is valid for non VLV/CHV platforms.

It is reporting the actual hw frequency. I still fail to see how
flushing the pending work is important for reporting that instantaneous
value.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Add gt_act_freq_mhz sysfs file

[Ville Syrjälä]

On Mon, Jan 26, 2015 at 08:42:56PM +0000, Chris Wilson wrote:
> On Mon, Jan 26, 2015 at 12:22:07PM -0800, O'Rourke, Tom wrote:
> > On Sun, Jan 25, 2015 at 09:34:33AM +0000, Chris Wilson wrote:
> > > On Fri, Jan 23, 2015 at 09:04:24PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Currently the 'gt_cur_freq_mhz' file shows the actual GPU frequency on
> > > > VLV/CHV, and the last requested frequency on other platforms. Change the
> > > > meaning of the file on VLV/CHV to follow the the other platforms, and
> > > > introduce a new file 'gt_act_freq_mhz' which shows the actual frequency
> > > > on all platforms.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > 
> > > >  drivers/gpu/drm/i915/i915_sysfs.c | 35 ++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 34 insertions(+), 1 deletion(-)
> > > > 
> > > > +static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> > > > +				    struct device_attribute *attr, char *buf)
> > > > +{
> > > > +	struct drm_minor *minor = dev_to_drm_minor(kdev);
> > > > +	struct drm_device *dev = minor->dev;
> > > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > +	int ret;
> > > > +
> > > > +	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> > > 
> > > Is this required for querying the current value?
> > > 
> > > Though probably better to keep it similar to the others.
> > > -Chris
> > 
> > [TOR:] flush_delayed_work is needed to make sure rps.cur_freq
> > is valid for non VLV/CHV platforms.
> 
> It is reporting the actual hw frequency. I still fail to see how
> flushing the pending work is important for reporting that instantaneous
> value.

Yeah, I suppose it's not really needed for act_freq, but I don't see any
real harm in having it there.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Use intel_gpu_freq() and intel_freq_opcode()

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5641
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              353/353              352/353
ILK                                  353/353              353/353
SNB              +1-1              400/422              400/422
IVB              +2-1              485/487              486/487
BYT                                  296/296              296/296
HSW              +1-1              507/508              507/508
BDW                                  401/402              401/402
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_render_linear_blits      PASS(3, M25M23)      CRASH(1, M23)
*SNB  igt_kms_flip_event_leak      NSPT(3, M35M22)      PASS(1, M22)
*SNB  igt_kms_flip_tiling_flip-changes-tiling      PASS(2, M35M22)      FAIL(1, M22)
 IVB  igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance      DMESG_WARN(2, M34)PASS(3, M4)      PASS(1, M4)
 IVB  igt_gem_storedw_batches_loop_normal      DMESG_WARN(2, M34M4)PASS(5, M34M4M21)      PASS(1, M4)
*IVB  igt_gem_storedw_batches_loop_secure-dispatch      PASS(2, M34M4)      DMESG_WARN(1, M4)
 HSW  igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance      DMESG_WARN(1, M40)PASS(5, M40M20)      PASS(1, M20)
 HSW  igt_gem_storedw_loop_vebox      DMESG_WARN(1, M20)PASS(2, M40M20)      DMESG_WARN(1, M20)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx