Re: [Intel-gfx] [PATCH v4 07/26] drm/i915/slpc: Use intel_slpc_* functions if supported

[Kamble, Sagar A]

Thanks for the review.


On 9/9/2016 10:50 PM, Chris Wilson wrote:

On Fri, Sep 09, 2016 at 06:21:26PM +0530, Sagar Arun Kamble wrote:

@@ -6720,31 +6743,38 @@ void intel_enable_gt_powersave(struct drm_i915_private 
*dev_priv)
+   if (intel_slpc_enabled()) {
+   } else {
  
-	WARN_ON(dev_priv->rps.max_freq < dev_priv->rps.min_freq);

-   WARN_ON(dev_priv->rps.idle_freq > dev_priv->rps.max_freq);
+   WARN_ON(dev_priv->rps.max_freq < dev_priv->rps.min_freq);
+   WARN_ON(dev_priv->rps.idle_freq > dev_priv->rps.max_freq);
  
-	WARN_ON(dev_priv->rps.efficient_freq < dev_priv->rps.min_freq);

-   WARN_ON(dev_priv->rps.efficient_freq > dev_priv->rps.max_freq);
+   WARN_ON(dev_priv->rps.efficient_freq < dev_priv->rps.min_freq);
+   WARN_ON(dev_priv->rps.efficient_freq > dev_priv->rps.max_freq);

You seem to be chickening out of some sanity checks on values we present
to the user.
-Chris


Will restore these. idle_freq not applicable to SLPC hence will remove 
check for it with SLPC.






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

Re: [Intel-gfx] [PATCH v4 07/26] drm/i915/slpc: Use intel_slpc_* functions if supported

[Chris Wilson]

On Fri, Sep 09, 2016 at 06:21:26PM +0530, Sagar Arun Kamble wrote:
> @@ -6720,31 +6743,38 @@ void intel_enable_gt_powersave(struct 
> drm_i915_private *dev_priv)
> + if (intel_slpc_enabled()) {
> + } else {
>  
> - WARN_ON(dev_priv->rps.max_freq < dev_priv->rps.min_freq);
> - WARN_ON(dev_priv->rps.idle_freq > dev_priv->rps.max_freq);
> + WARN_ON(dev_priv->rps.max_freq < dev_priv->rps.min_freq);
> + WARN_ON(dev_priv->rps.idle_freq > dev_priv->rps.max_freq);
>  
> - WARN_ON(dev_priv->rps.efficient_freq < dev_priv->rps.min_freq);
> - WARN_ON(dev_priv->rps.efficient_freq > dev_priv->rps.max_freq);
> + WARN_ON(dev_priv->rps.efficient_freq < dev_priv->rps.min_freq);
> + WARN_ON(dev_priv->rps.efficient_freq > dev_priv->rps.max_freq);

You seem to be chickening out of some sanity checks on values we present
to the user.
-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

[Intel-gfx] [PATCH v4 07/26] drm/i915/slpc: Use intel_slpc_* functions if supported

[Sagar Arun Kamble]

From: Tom O'Rourke 

On platforms with SLPC support: call intel_slpc_*()
functions from corresponding intel_*_gt_powersave()
functions; and do not use rps functions.

v1: Return void instead of ignored error code (Paulo)
enable/disable RC6 in SLPC flows (Sagar)
replace HAS_SLPC() use with intel_slpc_enabled()
or intel_slpc_active() (Paulo)
Fix for renaming gen9_disable_rps to gen9_disable_rc6 in
"drm/i915/bxt: Explicitly clear the Turbo control register"
Defer RC6 and SLPC enabling to intel_gen6_powersave_work. (Sagar)
Performance drop with SLPC was happening as ring frequency table
was not programmed when SLPC was enabled. This patch programs ring
frequency table with SLPC. Initial reset of SLPC is based on kernel
parameter as planning to add slpc state in intel_slpc_active. Cleanup
is also based on kernel parameter as SLPC gets disabled in
disable/suspend.(Sagar)

v2: Usage of INTEL_GEN instead of INTEL_INFO->gen (David)
Checkpatch update.

v3: Rebase

v4: Removed reset functions to comply with *_gt_powersave routines.
(Sagar)

Signed-off-by: Tom O'Rourke 
Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/Makefile |  3 +-
 drivers/gpu/drm/i915/intel_drv.h  |  4 ++
 drivers/gpu/drm/i915/intel_guc.h  |  1 +
 drivers/gpu/drm/i915/intel_pm.c   | 96 +++
 drivers/gpu/drm/i915/intel_slpc.c | 46 +++
 drivers/gpu/drm/i915/intel_slpc.h | 34 ++
 6 files changed, 153 insertions(+), 31 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
 create mode 100644 drivers/gpu/drm/i915/intel_slpc.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a7da246..229290d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -52,7 +52,8 @@ i915-y += i915_cmd_parser.o \
 
 # general-purpose microcontroller (GuC) support
 i915-y += intel_guc_loader.o \
- i915_guc_submission.o
+ i915_guc_submission.o \
+ intel_slpc.o
 
 # autogenerated null render state
 i915-y += intel_renderstate_gen6.o \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7868d5c..cf9aa24 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1705,6 +1705,10 @@ void chv_phy_powergate_lanes(struct intel_encoder 
*encoder,
 bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
  enum dpio_channel ch, bool override);
 
+static inline int intel_slpc_active(struct drm_i915_private *dev_priv)
+{
+   return 0;
+}
 
 /* intel_pm.c */
 void intel_init_clock_gating(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index d73e4ed..83dec66 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -27,6 +27,7 @@
 #include "intel_guc_fwif.h"
 #include "i915_guc_reg.h"
 #include "intel_ringbuffer.h"
+#include "intel_slpc.h"
 
 struct drm_i915_gem_request;
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 56bde62..db5c4ef 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4988,7 +4988,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
 * our rpm wakeref. And then disable the interrupts to stop any
 * futher RPS reclocking whilst we are asleep.
 */
-   gen6_disable_rps_interrupts(dev_priv);
+   if (!intel_slpc_active(dev_priv))
+   gen6_disable_rps_interrupts(dev_priv);
 
mutex_lock(_priv->rps.hw_lock);
if (dev_priv->rps.enabled) {
@@ -6641,6 +6642,9 @@ void intel_init_gt_powersave(struct drm_i915_private 
*dev_priv)
/* Finally allow us to boost to max by default */
dev_priv->rps.boost_freq = dev_priv->rps.max_freq;
 
+   if (intel_slpc_enabled())
+   intel_slpc_init(dev_priv);
+
mutex_unlock(_priv->rps.hw_lock);
mutex_unlock(_priv->drm.struct_mutex);
 
@@ -6649,7 +6653,9 @@ void intel_init_gt_powersave(struct drm_i915_private 
*dev_priv)
 
 void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
 {
-   if (IS_VALLEYVIEW(dev_priv))
+   if (intel_slpc_enabled())
+   intel_slpc_cleanup(dev_priv);
+   else if (IS_VALLEYVIEW(dev_priv))
valleyview_cleanup_gt_powersave(dev_priv);
 
if (!i915.enable_rc6)
@@ -6673,24 +6679,38 @@ void intel_suspend_gt_powersave(struct drm_i915_private 
*dev_priv)
intel_runtime_pm_put(dev_priv);
 
/* gen6_rps_idle() will be called later to disable interrupts */
+
+   if (intel_slpc_active(dev_priv))
+   intel_slpc_suspend(dev_priv);
 }
 
 void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv)
 {
-   dev_priv->rps.enabled = true; /* force disabling */