From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sun, Daisy" Subject: Re: [PATCH] drm/i915: Debugfs disable RPS boost and idle Date: Fri, 25 Apr 2014 14:16:36 -0700 Message-ID: <535AD0B4.5060605@intel.com> References: <1398446956-5245-1-git-send-email-daisy.sun@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1048673251==" Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 92CCE6EF9A for ; Fri, 25 Apr 2014 14:16:37 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============1048673251== Content-Type: multipart/alternative; boundary="------------070404090903030904020200" This is a multi-part message in MIME format. --------------070404090903030904020200 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Thanks for the timely reply, Daniel. The intention to bring debugfs for RPS boost and idle is when boost and=20 idle are disabled, we are able to have a clear vision of what normal=20 turbo algorithm. It=E2=80=98s very helpful to verify if the turbo algorithm is working as=20 expected, at least from the VPG validation team. Without the debugfs hooks, the RPS boost or idle may kicks in at anytime=20 and any circumstances, which makes it complicated. It does not mean RPS boost and idle is doing anything wrong, that's a=20 different story. A fixed frequency setting is useful in some other cases=20 but not enough to verify turbo algorithm. I'd like to add the purpose in commit msg if it's not clear. On 4/25/2014 10:41 AM, Daniel Vetter wrote: > On Fri, Apr 25, 2014 at 7:29 PM, Daisy Sun wrote: >> RP frequency request is affected by 2 modules: normal turbo >> algorithm and RPS boost algorithm. By adding RPS boost algorithm >> to the mix, the final frequency becomes relatively unpredictable. >> Add a switch to enable/disable RPS boost functionality. When >> disabled, RP frequency will follow the normal turbo algorithm only > This only really describes what the patch does, not why the rps boost > is a problem and why exactly we need to disable it. If you want to set > a fixed frequency then we have interfaces in sysfs for that, if the > booster does something stupid then we need to fix that. > -Daniel > >> Issue: VIZ-3801 >> Signed-off-by: Daisy Sun >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 40 +++++++++++++++++++++++++++= ++++++++++ >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_pm.c | 8 ++++++-- >> 3 files changed, 47 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i91= 5/i915_debugfs.c >> index 1e83ae4..2077bbd 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -3486,6 +3486,45 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops, >> i915_drop_caches_get, i915_drop_caches_set, >> "0x%08llx\n"); >> >> +static int i915_rps_disable_boost_get(void *data, u64 *val) >> +{ >> + struct drm_device *dev =3D data; >> + drm_i915_private *dev_priv =3D dev->dev_private; >> + >> + if (INTEL_INFO(dev)->gen < 6) >> + return -ENODEV; >> + >> + *val =3D dev_priv->rps.debugfs_disable_boost; >> + >> + return 0; >> +} >> + >> +static int i915_rps_disable_boost_set(void *data, u64 val) >> +{ >> + struct drm_device *dev =3D data; >> + drm_i915_private *dev_priv =3D dev->dev_private; >> + int ret; >> + >> + flush_delayed_work(&dev_priv->rps.delayed_resume_work); >> + >> + DRM_DEBUG_DRIVER("Setting RPS disable Boost-Idle mode to %s\n"= , >> + val ? "on" : "off"); >> + >> + ret =3D mutex_lock_interruptible(&dev_priv->rps.hw_lock); >> + if (ret) >> + return ret; >> + >> + dev_priv->rps.debugfs_disable_boost =3D val; >> + >> + mutex_unlock(&dev_priv->rps.hw_lock); >> + >> + return 0; >> +} >> + >> +DEFINE_SIMPLE_ATTRIBUTE(i915_rps_disable_boost_fops, >> + i915_rps_disable_boost_get, i915_rps_disable_boost_set= , >> + "%llu\n"); >> + >> static int >> i915_max_freq_get(void *data, u64 *val) >> { >> @@ -3821,6 +3860,7 @@ static const struct i915_debugfs_files { >> {"i915_wedged", &i915_wedged_fops}, >> {"i915_max_freq", &i915_max_freq_fops}, >> {"i915_min_freq", &i915_min_freq_fops}, >> + {"i915_rps_disable_boost", &i915_rps_disable_boost_fops}, >> {"i915_cache_sharing", &i915_cache_sharing_fops}, >> {"i915_ring_stop", &i915_ring_stop_fops}, >> {"i915_ring_missed_irq", &i915_ring_missed_irq_fops}, >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i9= 15_drv.h >> index 272aa7a..9c427da 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -847,6 +847,7 @@ struct intel_gen6_power_mgmt { >> int last_adj; >> enum { LOW_POWER, BETWEEN, HIGH_POWER } power; >> >> + bool debugfs_disable_boost; >> bool enabled; >> struct delayed_work delayed_resume_work; >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/in= tel_pm.c >> index 75c1c76..6acac14 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3163,7 +3163,9 @@ void gen6_rps_idle(struct drm_i915_private *dev_= priv) >> struct drm_device *dev =3D dev_priv->dev; >> >> mutex_lock(&dev_priv->rps.hw_lock); >> - if (dev_priv->rps.enabled) { >> + >> + if (dev_priv->rps.enabled >> + && !dev_priv->rps.debugfs_disable_boost) { >> if (IS_VALLEYVIEW(dev)) >> vlv_set_rps_idle(dev_priv); >> else >> @@ -3178,7 +3180,9 @@ void gen6_rps_boost(struct drm_i915_private *dev= _priv) >> struct drm_device *dev =3D dev_priv->dev; >> >> mutex_lock(&dev_priv->rps.hw_lock); >> - if (dev_priv->rps.enabled) { >> + >> + if (dev_priv->rps.enabled >> + && !dev_priv->rps.debugfs_disable_boost) { >> if (IS_VALLEYVIEW(dev)) >> valleyview_set_rps(dev_priv->dev, dev_priv->r= ps.max_freq_softlimit); >> else >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > --------------070404090903030904020200 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks for the timely reply, Daniel.

The intention to bring debugfs for RPS boost and idle is when boost and idle are disabled, we are able to have a clear vision of what normal turbo algorithm.
It=E2=80=98s very helpful to verify if the turbo algorithm is w= orking as expected, at least from the VPG validation team.

Without the debugfs hooks, the RPS boost or idle may kicks in at anytime and any circumstances, which makes it complicated. It does not mean RPS boost and idle is doing anything wrong, that's a different story. A fixed frequency setting is useful in some other cases but not enough to verify turbo algorithm.

I'd like to add the purpose in commit msg if it's not clear.

On 4/25/2014 10:41 AM, Daniel Vetter wrote:
On Fri, Apr 25, 2014 at 7:29 PM, Daisy Sun <daisy.sun=
@intel.com> wrote:

RP frequency request is affected by 2 modules: nor=
mal turbo
algorithm and RPS boost algorithm. By adding RPS boost algorithm
to the mix, the final frequency becomes relatively unpredictable.
Add a switch to enable/disable RPS boost functionality. When
disabled, RP frequency will follow the normal turbo algorithm only

This only really describes what the patch does, not why the rps boost
is a problem and why exactly we need to disable it. If you want to set
a fixed frequency then we have interfaces in sysfs for that, if the
booster does something stupid then we need to fix that.
-Daniel


Issue: VIZ-3801
Signed-off-by: Daisy Sun <daisy.sun@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 40 +++++++++++++++++++++++++++++++=
++++++
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c     |  8 ++++++--
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i=
915_debugfs.c
index 1e83ae4..2077bbd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3486,6 +3486,45 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops,
                        i915_drop_caches_get, i915_drop_caches_set,
                        "0x%08llx\n");

+static int i915_rps_disable_boost_get(void *data, u64 *val)
+{
+       struct drm_device *dev =3D data;
+       drm_i915_private *dev_priv =3D dev->dev_private;
+
+       if (INTEL_INFO(dev)->gen < 6)
+               return -ENODEV;
+
+       *val =3D dev_priv->rps.debugfs_disable_boost;
+
+       return 0;
+}
+
+static int i915_rps_disable_boost_set(void *data, u64 val)
+{
+       struct drm_device *dev =3D data;
+       drm_i915_private *dev_priv =3D dev->dev_private;
+       int ret;
+
+       flush_delayed_work(&dev_priv->rps.delayed_resume_work);
+
+       DRM_DEBUG_DRIVER("Setting RPS disable Boost-Idle mode to %s\n",
+                                val ? "on" : "off");
+
+       ret =3D mutex_lock_interruptible(&dev_priv->rps.hw_lock);
+       if (ret)
+               return ret;
+
+       dev_priv->rps.debugfs_disable_boost =3D val;
+
+       mutex_unlock(&dev_priv->rps.hw_lock);
+
+       return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_rps_disable_boost_fops,
+               i915_rps_disable_boost_get, i915_rps_disable_boost_set,
+                       "%llu\n");
+
 static int
 i915_max_freq_get(void *data, u64 *val)
 {
@@ -3821,6 +3860,7 @@ static const struct i915_debugfs_files {
        {"i915_wedged", &i915_wedged_fops},
        {"i915_max_freq", &i915_max_freq_fops},
        {"i915_min_freq", &i915_min_freq_fops},
+       {"i915_rps_disable_boost", &i915_rps_disable_boost_fops},
        {"i915_cache_sharing", &i915_cache_sharing_fops},
        {"i915_ring_stop", &i915_ring_stop_fops},
        {"i915_ring_missed_irq", &i915_ring_missed_irq_fops},
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_=
drv.h
index 272aa7a..9c427da 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -847,6 +847,7 @@ struct intel_gen6_power_mgmt {
        int last_adj;
        enum { LOW_POWER, BETWEEN, HIGH_POWER } power;

+       bool debugfs_disable_boost;
        bool enabled;
        struct delayed_work delayed_resume_work;

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel=
_pm.c
index 75c1c76..6acac14 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3163,7 +3163,9 @@ void gen6_rps_idle(struct drm_i915_private *dev_pri=
v)
        struct drm_device *dev =3D dev_priv->dev;

        mutex_lock(&dev_priv->rps.hw_lock);
-       if (dev_priv->rps.enabled) {
+
+       if (dev_priv->rps.enabled
+               && !dev_priv->rps.debugfs_disable_boost) {
                if (IS_VALLEYVIEW(dev))
                        vlv_set_rps_idle(dev_priv);
                else
@@ -3178,7 +3180,9 @@ void gen6_rps_boost(struct drm_i915_private *dev_pr=
iv)
        struct drm_device *dev =3D dev_priv->dev;

        mutex_lock(&dev_priv->rps.hw_lock);
-       if (dev_priv->rps.enabled) {
+
+       if (dev_priv->rps.enabled
+               && !dev_priv->rps.debugfs_disable_boost) {
                if (IS_VALLEYVIEW(dev))
                        valleyview_set_rps(dev_priv->dev, dev_priv->=
;rps.max_freq_softlimit);
                else
--
1.9.1

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

--------------070404090903030904020200-- --===============1048673251== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1048673251==--