On Tue, Apr 16, 2019 at 08:30:22AM -0700, Bob Paauwe wrote:
On Mon, 15 Apr 2019 17:33:30 -0700
Vanshidhar Konda <[email protected]> wrote:
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 <[email protected]>
>---
> 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.
Using u32 is consistent with all the other _store functions in the file
and changing it would also mean changing the kstrtou32 call below. I'd
rather this function stay consistent with the min/max/boost frequency
functions. Changing to u8 would be a separate change and should be
applied to all the similar functions.
Thanks for pointing that out.
I recently joined the team, so not sure if you would like someone else
to review as well.
Reviewed-by: Vanshidhar Konda <[email protected]>
>+ 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.
Similar to above, this is consistent with the other similar functions.
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
>[email protected]
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
--
Bob Paauwe
[email protected]
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
