Re: [PATCH] drm/i915: Better overclock support

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Ben Widawsky <ben@bwidawsk.net> writes:

> Most importantly this will allow users to set overclock frequencies in
> sysfs. Previously the max was limited by the RP0 max as opposed to the
> overclock max. This is useful if one wants to either limit the max
> overclock frequency, or set the minimum frequency to be in the overclock
> range. It also fixes an issue where if one sets the max frequency to be
> below the overclock max, they wouldn't be able to set back the proper
> overclock max.
>
> In addition I've added a couple of other bits:
> Show the overclock freq. as max in sysfs
> Print the overclock max in debugfs.
> Print a warning if the user sets the min frequency to be in the
> overclock range.
>
> In this patch I've decided to store the hw_max when we read it from the
> pcode at init. The reason I do this is the pcode reads can fail, and are
> slow.
>
> Reported-by: freezer?
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_sysfs.c   | 12 ++++++++----
>  drivers/gpu/drm/i915/intel_pm.c     |  3 ++-
>  4 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7df8351..f081bb3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1006,6 +1006,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>  		max_freq = rp_state_cap & 0xff;
>  		seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
>  			   max_freq * GT_FREQUENCY_MULTIPLIER);
> +
> +		seq_printf(m, "Max overclocked frequency: %dMHz\n",
> +			   dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER);
>  	} else {
>  		seq_printf(m, "no P-state info available\n");
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1657d873..9b53b39c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -648,6 +648,7 @@ struct intel_gen6_power_mgmt {
>  	u8 cur_delay;
>  	u8 min_delay;
>  	u8 max_delay;
> +	u8 hw_max;
>  
>  	struct delayed_work delayed_resume_work;
>  
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index a3a3e22..5faf1a7 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -226,7 +226,7 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
>  	int ret;
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> -	ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
> +	ret = dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER;
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
>  	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> @@ -251,7 +251,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -	hw_max = (rp_state_cap & 0xff);
> +	hw_max = dev_priv->rps.hw_max;
>  	hw_min = ((rp_state_cap & 0xff0000) >> 16);
>  
>  	if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay) {
> @@ -290,7 +290,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
>  	struct drm_device *dev = minor->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 val, rp_state_cap, hw_max, hw_min;
> +	u32 val, rp_state_cap, hw_max, hw_min, non_oc_max;
>  	ssize_t ret;
>  
>  	ret = kstrtou32(buf, 0, &val);
> @@ -302,7 +302,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -	hw_max = (rp_state_cap & 0xff);
> +	hw_max = dev_priv->rps.hw_max;
> +	non_oc_max = (rp_state_cap & 0xff);
>  	hw_min = ((rp_state_cap & 0xff0000) >> 16);
>  
>  	if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay) {
> @@ -310,6 +311,9 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  		return -EINVAL;
>  	}
>  
> +	if (val > non_oc_max)
> +		DRM_DEBUG("User selected overclocked frequency for min\n");
> +
>  	if (dev_priv->rps.cur_delay < val)
>  		gen6_set_rps(dev_priv->dev, val);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ce3db2c..2edb743 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2555,7 +2555,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>  	gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
>  
>  	/* In units of 100MHz */

Not a problem with this patch but the above comment should be
fixed as the freq is in units of 50Mhz right?

> -	dev_priv->rps.max_delay = rp_state_cap & 0xff;
> +	dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff;
>  	dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16;
>  	dev_priv->rps.cur_delay = 0;
>  
> @@ -2635,6 +2635,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>  			DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n",
>  					 (dev_priv->rps.max_delay & 0xff) * 50,
>  					 (pcu_mbox & 0xff) * 50);
> +			dev_priv->rps.hw_max = pcu_mbox & 0xff;
>  			dev_priv->rps.max_delay = pcu_mbox & 0xff;
>  		}
>  	} else {
> -- 
> 1.8.2

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>