Re: [PATCH] drm/i915: Debugfs disable RPS boost and idle

["Sun, Daisy" <daisy.sun@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5822 bytes --]

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‘s very helpful to verify if the turbo algorithm is working 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: 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 <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/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 = data;
>> +       drm_i915_private *dev_priv = dev->dev_private;
>> +
>> +       if (INTEL_INFO(dev)->gen < 6)
>> +               return -ENODEV;
>> +
>> +       *val = dev_priv->rps.debugfs_disable_boost;
>> +
>> +       return 0;
>> +}
>> +
>> +static int i915_rps_disable_boost_set(void *data, u64 val)
>> +{
>> +       struct drm_device *dev = data;
>> +       drm_i915_private *dev_priv = 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 = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
>> +       if (ret)
>> +               return ret;
>> +
>> +       dev_priv->rps.debugfs_disable_boost = 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_priv)
>>          struct drm_device *dev = 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 = 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
>
>


[-- Attachment #1.2: Type: text/html, Size: 6705 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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