[Intel-gfx] [PATCH] drm/i915: Configurable GT idle frequency

[Vanshidhar Konda]

On Mon, Apr 15, 2019 at 04:05:26PM -0700, Bob Paauwe wrote:
>There are real-time use cases where having deterministic CPU processes
>can be more important than GPU power/performance. Parking the GPU at a
>specific freqency by setting idle, min and max prohibits the normal
>dynamic GPU frequency switching which can introduce significant PCI-E
>latency. This adds the ability to configure the GPU "idle" frequecy
>using the same method that already exists for minimum and maximum
>frequencies.
>
>In addition, parking the idle frequency may reduce spool up latencies
>on GPU workloads.
>
>Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
>---
> drivers/gpu/drm/i915/i915_sysfs.c | 60 +++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
>index 41313005af42..62612c23d514 100644
>--- a/drivers/gpu/drm/i915/i915_sysfs.c
>+++ b/drivers/gpu/drm/i915/i915_sysfs.c
>@@ -454,11 +454,69 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> 	return ret ?: count;
> }
>
>+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
>+{
>+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>+
>+	return snprintf(buf, PAGE_SIZE, "%d\n",
>+			intel_gpu_freq(dev_priv,
>+				       dev_priv->gt_pm.rps.idle_freq));
>+}
>+
>+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
>+				      struct device_attribute *attr,
>+				      const char *buf, size_t count)
>+{
>+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>+	struct intel_rps *rps = &dev_priv->gt_pm.rps;
>+	intel_wakeref_t wakeref;
>+	u32 val;

val can probably just be u8. max_freq, min_freq, etc. are only u8 in
struct intel_rps *rps.

>+	ssize_t ret;
>+
>+	ret = kstrtou32(buf, 0, &val);
>+	if (ret)
>+		return ret;
>+
>+	wakeref = intel_runtime_pm_get(dev_priv);
>+
>+	mutex_lock(&dev_priv->pcu_lock);
>+
>+	val = intel_freq_opcode(dev_priv, val);
>+
>+	if (val < rps->min_freq ||
>+	    val > rps->max_freq ||
>+	    val > rps->max_freq_softlimit) {
>+		mutex_unlock(&dev_priv->pcu_lock);
>+		intel_runtime_pm_put(dev_priv, wakeref);
>+		return -EINVAL;
>+	}
>+
>+	rps->idle_freq = val;
>+
>+	val = clamp_t(int, rps->cur_freq,
>+		      rps->idle_freq,
>+		      rps->max_freq_softlimit);

This should probably be clamped to u8 instead of int.

Vanshi

>+
>+	/*
>+	 * If the current freq is at the old idle freq we should
>+	 * ajust it to the new idle.  Calling *_set_rps will also
>+	 * update the interrupt limits and PMINTRMSK if ncessary.
>+	 */
>+	ret = intel_set_rps(dev_priv, val);
>+
>+	mutex_unlock(&dev_priv->pcu_lock);
>+
>+	intel_runtime_pm_put(dev_priv, wakeref);
>+
>+	return ret ?: count;
>+}
>+
> static DEVICE_ATTR_RO(gt_act_freq_mhz);
> static DEVICE_ATTR_RO(gt_cur_freq_mhz);
> static DEVICE_ATTR_RW(gt_boost_freq_mhz);
> static DEVICE_ATTR_RW(gt_max_freq_mhz);
> static DEVICE_ATTR_RW(gt_min_freq_mhz);
>+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
>
> static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
>
>@@ -492,6 +550,7 @@ static const struct attribute * const gen6_attrs[] = {
> 	&dev_attr_gt_boost_freq_mhz.attr,
> 	&dev_attr_gt_max_freq_mhz.attr,
> 	&dev_attr_gt_min_freq_mhz.attr,
>+	&dev_attr_gt_idle_freq_mhz.attr,
> 	&dev_attr_gt_RP0_freq_mhz.attr,
> 	&dev_attr_gt_RP1_freq_mhz.attr,
> 	&dev_attr_gt_RPn_freq_mhz.attr,
>@@ -504,6 +563,7 @@ static const struct attribute * const vlv_attrs[] = {
> 	&dev_attr_gt_boost_freq_mhz.attr,
> 	&dev_attr_gt_max_freq_mhz.attr,
> 	&dev_attr_gt_min_freq_mhz.attr,
>+	&dev_attr_gt_idle_freq_mhz.attr,
> 	&dev_attr_gt_RP0_freq_mhz.attr,
> 	&dev_attr_gt_RP1_freq_mhz.attr,
> 	&dev_attr_gt_RPn_freq_mhz.attr,
>-- 
>2.19.2
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx