Re: [Intel-gfx] drm/i915/slpc: Update current requested frequency

[David Weinehall]

On Sat, Aug 20, 2016 at 10:39:10AM +0530, Sagar Arun Kamble wrote:
> From: Tom O'Rourke 
> 
> 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 
> Signed-off-by: Sagar Arun Kamble 
> ---
>  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   | 8 
>  4 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 01ae5ee..a99a3f6 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>  
>   intel_runtime_pm_get(dev_priv);
>  
> + if (intel_slpc_active(dev_priv))
> + 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);
> @@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void 
> *data)
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   struct drm_file *file;
>  
> + if (intel_slpc_active(dev_priv))
> + 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? %s [%x]\n",
>  yesno(dev_priv->gt.awake), dev_priv->gt.active_engines);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 764fad0..fcd2e98 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, 
> unsigned long len);
>   __T;\
>  })
>  
> +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 d4adf28..1654245 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6997,6 +6997,7 @@ enum {
>  #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 f1ffde7..8404816 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>   struct drm_device *dev = minor->dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
>  
> + if (intel_slpc_active(dev_priv)) {
> + intel_runtime_pm_get(dev_priv);
> + mutex_lock(_priv->rps.hw_lock);
> + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);
> + mutex_unlock(_priv->rps.hw_lock);
> + intel_runtime_pm_put(dev_priv);
> + }
> +
>   return snprintf(buf, PAGE_SIZE, "%d\n",
>   intel_gpu_freq(dev_priv,
>  dev_priv->rps.cur_freq));

Reviewed-by: David Weinehall 
-- 
 /) David Weinehall  /) Northern lights wander  (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/(/   Full colour fire   (/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] drm/i915/slpc: Update current requested frequency

[Kamble, Sagar A]

On 8/20/2016 1:45 PM, Chris Wilson wrote:

On Sat, Aug 20, 2016 at 10:39:10AM +0530, Sagar Arun Kamble wrote:

From: Tom O'Rourke 

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 
Signed-off-by: Sagar Arun Kamble 
---
  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   | 8 
  4 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 01ae5ee..a99a3f6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void 
*unused)
  
  	intel_runtime_pm_get(dev_priv);
  
+	if (intel_slpc_active(dev_priv))

+   dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);

Do not alter cur_freq here, as we print out RPNSWEQ and updating
cur_freq defeats the purpose of showing the internal value vs the hw
value.

Instead add "SLPC active" to the output.

Fixed in the next series.



+
if (IS_GEN5(dev)) {
u16 rgvswctl = I915_READ16(MEMSWCTL);
u16 rgvstat = I915_READ16(MEMSTAT_ILK);
@@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void 
*data)
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_file *file;

if (intel_slpc_active(dev_priv))
return -ENODEV;

Fixed in the next series.
  
+	if (intel_slpc_active(dev_priv))

+   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? %s [%x]\n",
   yesno(dev_priv->gt.awake), dev_priv->gt.active_engines);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 764fad0..fcd2e98 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, 
unsigned long len);
__T;\
  })
  
+static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv)

+{
+   return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ));
+}

Removed in the next series.

Move to sysfs and look carefully at what you wrote.


  #endif
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d4adf28..1654245 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6997,6 +6997,7 @@ enum {
  #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 f1ffde7..8404816 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
struct drm_device *dev = minor->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
  
+	if (intel_slpc_active(dev_priv)) {

+   intel_runtime_pm_get(dev_priv);

Use get_if_in_use and just show a stale value when the hw is asleep
would be my preference. cur_freq is just our request, act_freq is the
actual hw value.
With cur_freq not making sense currently with SLPC, I am removing this 
altogether and adding new sysfs interface

for knowing HW requested frequency which will be ideally SLPC requested.



+   mutex_lock(_priv->rps.hw_lock);

Useless mutex.


+   dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);
+   mutex_unlock(_priv->rps.hw_lock);
+   intel_runtime_pm_put(dev_priv);
+   }
+
return snprintf(buf, PAGE_SIZE, "%d\n",
intel_gpu_freq(dev_priv,
   dev_priv->rps.cur_freq));
--
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


___
Intel-gfx mailing list

Re: [Intel-gfx] drm/i915/slpc: Update current requested frequency

[Chris Wilson]

On Sat, Aug 20, 2016 at 10:39:10AM +0530, Sagar Arun Kamble wrote:
> From: Tom O'Rourke 
> 
> 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 
> Signed-off-by: Sagar Arun Kamble 
> ---
>  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   | 8 
>  4 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 01ae5ee..a99a3f6 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>  
>   intel_runtime_pm_get(dev_priv);
>  
> + if (intel_slpc_active(dev_priv))
> + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);

Do not alter cur_freq here, as we print out RPNSWEQ and updating
cur_freq defeats the purpose of showing the internal value vs the hw
value.

Instead add "SLPC active" to the output.

> +
>   if (IS_GEN5(dev)) {
>   u16 rgvswctl = I915_READ16(MEMSWCTL);
>   u16 rgvstat = I915_READ16(MEMSTAT_ILK);
> @@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void 
> *data)
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   struct drm_file *file;

if (intel_slpc_active(dev_priv))
return -ENODEV;
>  
> + if (intel_slpc_active(dev_priv))
> + 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? %s [%x]\n",
>  yesno(dev_priv->gt.awake), dev_priv->gt.active_engines);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 764fad0..fcd2e98 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, 
> unsigned long len);
>   __T;\
>  })
>  
> +static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv)
> +{
> + return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ));
> +}

Move to sysfs and look carefully at what you wrote.

>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d4adf28..1654245 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6997,6 +6997,7 @@ enum {
>  #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 f1ffde7..8404816 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>   struct drm_device *dev = minor->dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
>  
> + if (intel_slpc_active(dev_priv)) {
> + intel_runtime_pm_get(dev_priv);

Use get_if_in_use and just show a stale value when the hw is asleep
would be my preference. cur_freq is just our request, act_freq is the
actual hw value.

> + mutex_lock(_priv->rps.hw_lock);

Useless mutex.

> + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);
> + mutex_unlock(_priv->rps.hw_lock);
> + intel_runtime_pm_put(dev_priv);
> + }
> +
>   return snprintf(buf, PAGE_SIZE, "%d\n",
>   intel_gpu_freq(dev_priv,
>  dev_priv->rps.cur_freq));
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Intel-gfx] drm/i915/slpc: Update current requested frequency

[Sagar Arun Kamble]

From: Tom O'Rourke 

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 
Signed-off-by: Sagar Arun Kamble 
---
 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   | 8 
 4 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 01ae5ee..a99a3f6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void 
*unused)
 
intel_runtime_pm_get(dev_priv);
 
+   if (intel_slpc_active(dev_priv))
+   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);
@@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void 
*data)
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_file *file;
 
+   if (intel_slpc_active(dev_priv))
+   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? %s [%x]\n",
   yesno(dev_priv->gt.awake), dev_priv->gt.active_engines);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 764fad0..fcd2e98 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, 
unsigned long len);
__T;\
 })
 
+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 d4adf28..1654245 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6997,6 +6997,7 @@ enum {
 #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 f1ffde7..8404816 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
struct drm_device *dev = minor->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
 
+   if (intel_slpc_active(dev_priv)) {
+   intel_runtime_pm_get(dev_priv);
+   mutex_lock(_priv->rps.hw_lock);
+   dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv);
+   mutex_unlock(_priv->rps.hw_lock);
+   intel_runtime_pm_put(dev_priv);
+   }
+
return snprintf(buf, PAGE_SIZE, "%d\n",
intel_gpu_freq(dev_priv,
   dev_priv->rps.cur_freq));
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx