Re: [PATCH 10/21] drm/i915/slpc: Update current requested frequency

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, Apr 27, 2016 at 06:10:54PM -0700, tom.orourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> When SLPC is controlling requested frequency, the rps.cur_freq
> value is not used to make the frequency request.
> 
> Before using rps.cur_freq in sysfs or debugfs, read
> requested frequency from register to get the value
> most recently requested by SLPC firmware.
> 
> v2: replace HAS_SLPC with intel_slpc_active (Paulo)
> v3: Avoid magic numbers (Nick)
>     Use a function for repeated code (Jon)
> 
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
>  drivers/gpu/drm/i915/i915_drv.h     | 5 +++++
>  drivers/gpu/drm/i915/i915_reg.h     | 1 +
>  drivers/gpu/drm/i915/i915_sysfs.c   | 3 +++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8b8d6f0..1295d8b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1168,6 +1168,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>  
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> +	if (intel_slpc_active(dev))
> +		dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);

No. This must remain our own cur_freq.

What you want to report is the HW current requested frequency, but the
reporting of our SW state must remain just that.

Do not fudge our bookkeeping when reporting it, you are just papering
over the very bug it is trying to report.

> +
>  	if (IS_GEN5(dev)) {
>  		u16 rgvswctl = I915_READ16(MEMSWCTL);
>  		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
> @@ -2399,6 +2402,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_file *file;
>  
> +	if (intel_slpc_active(dev))
> +		dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);

This is just silly since this whole mechanism is nerfed.

> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 2d576b7..826e40c 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -318,6 +318,9 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>  	intel_runtime_pm_get(dev_priv);
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> +	if (intel_slpc_active(dev))
> +		dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);

We don't have a sysfs for reporting the HW requested frequency. If you
make the change here for SLPC, make the change for the other gen as
well. It's the important distinction between using cur_freq and RPNSWREQ.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx