From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency
Date: Mon, 11 Jul 2016 14:39:19 +0300 [thread overview]
Message-ID: <87poqktndk.fsf@intel.com> (raw)
In-Reply-To: <1468055535-19740-7-git-send-email-chris@chris-wilson.co.uk>
Chris Wilson <chris@chris-wilson.co.uk> writes:
> To allow the user finer control over waitboosting, allow them to set the
> frequency we request for the boost. This also them allows to effectively
> disable the boosting by setting the boost request to a low frequency.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 9 +++++----
> drivers/gpu/drm/i915/i915_sysfs.c | 38 +++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
> 5 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 844fea795bae..d1ff4cb9b90e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1381,6 +1381,8 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
> intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
> seq_printf(m, "Min freq: %d MHz\n",
> intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
> + seq_printf(m, "Boost freq: %d MHz\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
> seq_printf(m, "Max freq: %d MHz\n",
> intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
> seq_printf(m,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4478cc852489..bb59bc637f7b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1170,6 +1170,7 @@ struct intel_gen6_power_mgmt {
> u8 max_freq_softlimit; /* Max frequency permitted by the driver */
> u8 max_freq; /* Maximum frequency, RP0 if not overclocking */
> u8 min_freq; /* AKA RPn. Minimum frequency */
> + u8 boost_freq; /* Frequency to request when wait boosting */
> u8 idle_freq; /* Frequency to request when we are idle */
> u8 efficient_freq; /* AKA RPe. Pre-determined balanced frequency */
> u8 rp1_freq; /* "less than" RP0 power/freqency */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1c2aec392412..c8ed36765301 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1105,9 +1105,10 @@ static void gen6_pm_rps_work(struct work_struct *work)
> new_delay = dev_priv->rps.cur_freq;
> min = dev_priv->rps.min_freq_softlimit;
> max = dev_priv->rps.max_freq_softlimit;
> -
> - if (client_boost) {
> - new_delay = dev_priv->rps.max_freq_softlimit;
> + if (client_boost || any_waiters(dev_priv))
> + max = dev_priv->rps.max_freq;
> + if (client_boost && new_delay < dev_priv->rps.boost_freq) {
> + new_delay = dev_priv->rps.boost_freq;
What should be the rules for boost_freq? With this change it can
be over max_freq_softlimit and thus will break pm_rps.
-Mika
> adj = 0;
> } else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
> if (adj > 0)
> @@ -1122,7 +1123,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
> new_delay = dev_priv->rps.efficient_freq;
> adj = 0;
> }
> - } else if (any_waiters(dev_priv)) {
> + } else if (client_boost || any_waiters(dev_priv)) {
> adj = 0;
> } else if (pm_iir & GEN6_PM_RP_DOWN_TIMEOUT) {
> if (dev_priv->rps.cur_freq > dev_priv->rps.efficient_freq)
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index d61829e54f93..8c045ff47f0e 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -318,6 +318,41 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> }
>
> +static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> + struct drm_minor *minor = dev_to_drm_minor(kdev);
> + struct drm_i915_private *dev_priv = to_i915(minor->dev);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
> +}
> +
> +static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct drm_minor *minor = dev_to_drm_minor(kdev);
> + struct drm_device *dev = minor->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 val;
> + ssize_t ret;
> +
> + ret = kstrtou32(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + /* Validate against (static) hardware limits */
> + val = intel_freq_opcode(dev_priv, val);
> + if (val < dev_priv->rps.min_freq || val > dev_priv->rps.max_freq)
> + return -EINVAL;
> +
> + mutex_lock(&dev_priv->rps.hw_lock);
> + dev_priv->rps.boost_freq = val;
> + mutex_unlock(&dev_priv->rps.hw_lock);
> +
> + return count;
> +}
> +
> static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
> struct device_attribute *attr, char *buf)
> {
> @@ -465,6 +500,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_boost_freq_mhz, S_IRUGO, gt_boost_freq_mhz_show, gt_boost_freq_mhz_store);
> 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);
>
> @@ -498,6 +534,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_boost_freq_mhz.attr,
> &dev_attr_gt_max_freq_mhz.attr,
> &dev_attr_gt_min_freq_mhz.attr,
> &dev_attr_gt_RP0_freq_mhz.attr,
> @@ -509,6 +546,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_boost_freq_mhz.attr,
> &dev_attr_gt_max_freq_mhz.attr,
> &dev_attr_gt_min_freq_mhz.attr,
> &dev_attr_gt_RP0_freq_mhz.attr,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 24b23a51c56b..aab1e0b5d2eb 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4911,7 +4911,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
> */
> if (!(dev_priv->gt.awake &&
> dev_priv->rps.enabled &&
> - dev_priv->rps.cur_freq < dev_priv->rps.max_freq_softlimit))
> + dev_priv->rps.cur_freq < dev_priv->rps.boost_freq))
> return;
>
> /* Force a RPS boost (and don't count it against the client) if
> @@ -6532,6 +6532,9 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
> }
> }
>
> + /* Finally allow us to boost to max by default */
> + dev_priv->rps.boost_freq = dev_priv->rps.max_freq;
> +
> mutex_unlock(&dev_priv->rps.hw_lock);
> }
>
> --
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-07-11 11:39 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-09 9:12 Mika's rps/rc6 fixes Chris Wilson
2016-07-09 9:12 ` [PATCH 01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping Chris Wilson
2016-07-11 11:42 ` Mika Kuoppala
2016-07-09 9:12 ` [PATCH 02/10] drm/i915: Kick hangcheck from retire worker Chris Wilson
2016-07-11 12:21 ` Mika Kuoppala
2016-07-11 12:46 ` Chris Wilson
2016-07-09 9:12 ` [PATCH 03/10] drm/i915: Preserve current RPS frequency across init Chris Wilson
2016-07-09 9:12 ` [PATCH 04/10] drm/i915: Perform static RPS frequency setup before userspace Chris Wilson
2016-07-11 13:31 ` Mika Kuoppala
2016-07-09 9:12 ` [PATCH 05/10] drm/i915: Move overclocking detection to alongside RPS frequency detection Chris Wilson
2016-07-11 13:37 ` Mika Kuoppala
2016-07-09 9:12 ` [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency Chris Wilson
2016-07-11 11:39 ` Mika Kuoppala [this message]
2016-07-11 11:49 ` Chris Wilson
2016-07-11 12:55 ` Mika Kuoppala
2016-07-11 13:09 ` Chris Wilson
2016-07-09 9:12 ` [PATCH 07/10] drm/i915: Remove superfluous powersave work flushing Chris Wilson
2016-07-11 14:11 ` Mika Kuoppala
2016-07-09 9:12 ` [PATCH 08/10] drm/i915: Defer enabling rc6 til after we submit the first batch/context Chris Wilson
2016-07-11 14:14 ` Mika Kuoppala
2016-07-11 14:24 ` Chris Wilson
2016-07-09 9:12 ` [PATCH 09/10] drm/i915: Hide gen6_update_ring_freq() Chris Wilson
2016-07-09 9:12 ` [PATCH 10/10] drm/i915: Remove temporary RPM wakeref assert disables Chris Wilson
2016-07-11 15:30 ` Mika Kuoppala
2016-07-09 9:58 ` ✗ Ro.CI.BAT: failure for series starting with [01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping Patchwork
2016-07-09 10:08 ` Chris Wilson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87poqktndk.fsf@intel.com \
--to=mika.kuoppala@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox