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

Submitted by tom.orourke@intel.com on April 28, 2016, 1:10 a.m.

Details

Message ID 1461805865-212590-11-git-send-email-tom.orourke@intel.com
State New
Headers show
Series "Add support for GuC-based SLPC" ( rev: 4 ) in Intel GFX

Not browsing as part of any series.

Commit Message

tom.orourke@intel.com April 28, 2016, 1:10 a.m.
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(+)

Patch hide | download patch | download mbox

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);
+
 	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);
+
 	seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled);
 	seq_printf(m, "GPU busy? %d\n", dev_priv->mm.busy);
 	seq_printf(m, "CPU waiting? %d\n", count_irq_waiters(dev_priv));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 393da67..55d31f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3728,4 +3728,9 @@  static inline void i915_trace_irq_get(struct intel_engine_cs *engine,
 		i915_gem_request_assign(&engine->trace_irq_req, req);
 }
 
+static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv)
+{
+	return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ));
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a2fd30..a7beb10 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6956,6 +6956,7 @@  enum skl_disp_power_wells {
 #define   GEN6_FREQUENCY(x)			((x)<<25)
 #define   HSW_FREQUENCY(x)			((x)<<24)
 #define   GEN9_FREQUENCY(x)			((x)<<23)
+#define   GEN9_GET_FREQUENCY(x)			((x)>>23)
 #define   GEN6_OFFSET(x)			((x)<<19)
 #define   GEN6_AGGRESSIVE_TURBO			(0<<15)
 #define GEN6_RC_VIDEO_FREQ			_MMIO(0xA00C)
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);
+
 	ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 

Comments

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