Re: [Intel-gfx] [PATCH 4/8] drm/i915/skl: Assert the requirements to enter or exit DC5.

2015-04-02 Thread Imre Deak 
On Wed, 2015-04-01 at 16:22 +0530, Animesh Manna wrote:
 From: Suketu Shah suketu.j.s...@intel.com
 
 Warn if the conditions to enter or exit DC5 are not satisfied such
 as support for runtime PM, state of power well, CSR loading etc.
 
 v2: Removed camelcase in functions and variables.
 
 v3: Do some minimal check to assert if CSR program is not loaded.
 
 v4:
 1] Used an appropriate function lookup_power_well() to identify power well,
 instead of using a magic number which can change in future.
 2] Split the conditions further in assert_can_enable_DC5() and added more 
 checks.
 3] Removed all WARNs from assert_can_disable_DC5 as they were unnecessary and 
 added two
new ones.
 4] Changed variable names as updated in earlier patches.
 
 v5:
 1] Change lookup_power_well function to take an int power well id.
 2] Define a new intel_display_power_well_is_enabled helper function to check 
 whether a
particular power well is enabled.
 3] Use CSR-related mutex in assert_csr_loaded function.
 
 v6: Remove use of dc5_enabled variable as it's no longer needed.
 
 v7:
 1] Rebase to latest.
 2] Move all DC5-related functions from intel_display.c to intel_runtime_pm.c.
 
 v8: After adding dmc ver 1.0 support rebased on top of nightly. (Animesh)
 
 Issue: VIZ-2819
 Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com
 Signed-off-by: Suketu Shah suketu.j.s...@intel.com
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com
 Signed-off-by: Animesh Manna animesh.ma...@intel.com
 ---
  drivers/gpu/drm/i915/intel_drv.h|  2 ++
  drivers/gpu/drm/i915/intel_runtime_pm.c | 61 
 ++---
  2 files changed, 58 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 39cb2dc..9aae624 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -1232,6 +1232,8 @@ void intel_power_domains_fini(struct drm_i915_private 
 *);
  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
  
 +bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
 + int power_well_id);

I haven't seen this being used outside of intel_runtime_pm, so no need
to export it.

  bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
   enum intel_display_power_domain domain);
  bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
 diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
 b/drivers/gpu/drm/i915/intel_runtime_pm.c
 index 8b917e2..f62d42b 100644
 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
 +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
 @@ -335,12 +335,52 @@ static void gen9_set_dc_state_debugmask_memory_up(
   }
  }
  
 -static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 +static void assert_csr_loaded(struct drm_i915_private *dev_priv)
 +{
 + mutex_lock(dev_priv-csr_lock);

No point in taking the lock here.

 +
 + WARN(!dev_priv-csr.loaded, CSR is not loaded.\n);
 + WARN(!I915_READ(CSR_PROGRAM_BASE),
 + CSR program storage start is NULL\n);
 + WARN(!I915_READ(CSR_SSP_BASE), CSR SSP Base Not fine\n);
 + WARN(!I915_READ(CSR_HTP_SKL), CSR HTP Not fine\n);
 +
 + mutex_unlock(dev_priv-csr_lock);
 +}
 +
 +static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
  {
   struct drm_device *dev = dev_priv-dev;
 + bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
 + SKL_DISP_PW_2);
 +
 + WARN(!IS_SKYLAKE(dev), Platform doesn't support DC5.\n);
 + WARN(!HAS_RUNTIME_PM(dev), Runtime PM not enabled.\n);
 + WARN(pg2_enabled, PG2 not disabled to enable DC5.\n);
 +
 + WARN((I915_READ(DC_STATE_EN)  DC_STATE_EN_UPTO_DC5),
 + DC5 already programmed to be enabled.\n);
 + WARN(dev_priv-pm.suspended,
 + DC5 cannot be enabled, if platform is runtime-suspended.\n);
 +
 + assert_csr_loaded(dev_priv);
 +}
 +
 +static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
 +{
 + bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
 + SKL_DISP_PW_2);
 +
 + WARN(!pg2_enabled, PG2 not enabled to disable DC5.\n);
 + WARN(dev_priv-pm.suspended,
 + Disabling of DC5 while platform is runtime-suspended should 
 never happen.\n);
 +}
 +
 +static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 +{
   uint32_t val;
  
 - WARN_ON(!IS_GEN9(dev));
 + assert_can_enable_dc5(dev_priv);
  
   DRM_DEBUG_KMS(Enabling DC5\n);
  
 @@ -355,10 +395,9 @@ static void gen9_enable_dc5(struct drm_i915_private 
 *dev_priv)
  
  static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
  {
 - struct drm_device *dev = dev_priv-dev;
   uint32_t val;
  
 -

[Intel-gfx] [PATCH 4/8] drm/i915/skl: Assert the requirements to enter or exit DC5.

2015-04-01 Thread Animesh Manna 
From: Suketu Shah suketu.j.s...@intel.com

Warn if the conditions to enter or exit DC5 are not satisfied such
as support for runtime PM, state of power well, CSR loading etc.

v2: Removed camelcase in functions and variables.

v3: Do some minimal check to assert if CSR program is not loaded.

v4:
1] Used an appropriate function lookup_power_well() to identify power well,
instead of using a magic number which can change in future.
2] Split the conditions further in assert_can_enable_DC5() and added more 
checks.
3] Removed all WARNs from assert_can_disable_DC5 as they were unnecessary and 
added two
   new ones.
4] Changed variable names as updated in earlier patches.

v5:
1] Change lookup_power_well function to take an int power well id.
2] Define a new intel_display_power_well_is_enabled helper function to check 
whether a
   particular power well is enabled.
3] Use CSR-related mutex in assert_csr_loaded function.

v6: Remove use of dc5_enabled variable as it's no longer needed.

v7:
1] Rebase to latest.
2] Move all DC5-related functions from intel_display.c to intel_runtime_pm.c.

v8: After adding dmc ver 1.0 support rebased on top of nightly. (Animesh)

Issue: VIZ-2819
Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Suketu Shah suketu.j.s...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
---
 drivers/gpu/drm/i915/intel_drv.h|  2 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 61 ++---
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 39cb2dc..9aae624 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1232,6 +1232,8 @@ void intel_power_domains_fini(struct drm_i915_private *);
 void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
 
+bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
+   int power_well_id);
 bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain);
 bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8b917e2..f62d42b 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -335,12 +335,52 @@ static void gen9_set_dc_state_debugmask_memory_up(
}
 }
 
-static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
+static void assert_csr_loaded(struct drm_i915_private *dev_priv)
+{
+   mutex_lock(dev_priv-csr_lock);
+
+   WARN(!dev_priv-csr.loaded, CSR is not loaded.\n);
+   WARN(!I915_READ(CSR_PROGRAM_BASE),
+   CSR program storage start is NULL\n);
+   WARN(!I915_READ(CSR_SSP_BASE), CSR SSP Base Not fine\n);
+   WARN(!I915_READ(CSR_HTP_SKL), CSR HTP Not fine\n);
+
+   mutex_unlock(dev_priv-csr_lock);
+}
+
+static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
 {
struct drm_device *dev = dev_priv-dev;
+   bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
+   SKL_DISP_PW_2);
+
+   WARN(!IS_SKYLAKE(dev), Platform doesn't support DC5.\n);
+   WARN(!HAS_RUNTIME_PM(dev), Runtime PM not enabled.\n);
+   WARN(pg2_enabled, PG2 not disabled to enable DC5.\n);
+
+   WARN((I915_READ(DC_STATE_EN)  DC_STATE_EN_UPTO_DC5),
+   DC5 already programmed to be enabled.\n);
+   WARN(dev_priv-pm.suspended,
+   DC5 cannot be enabled, if platform is runtime-suspended.\n);
+
+   assert_csr_loaded(dev_priv);
+}
+
+static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
+{
+   bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
+   SKL_DISP_PW_2);
+
+   WARN(!pg2_enabled, PG2 not enabled to disable DC5.\n);
+   WARN(dev_priv-pm.suspended,
+   Disabling of DC5 while platform is runtime-suspended should 
never happen.\n);
+}
+
+static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
+{
uint32_t val;
 
-   WARN_ON(!IS_GEN9(dev));
+   assert_can_enable_dc5(dev_priv);
 
DRM_DEBUG_KMS(Enabling DC5\n);
 
@@ -355,10 +395,9 @@ static void gen9_enable_dc5(struct drm_i915_private 
*dev_priv)
 
 static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
 {
-   struct drm_device *dev = dev_priv-dev;
uint32_t val;
 
-   WARN_ON(!IS_GEN9(dev));
+   assert_can_disable_dc5(dev_priv);
 
DRM_DEBUG_KMS(Disabling DC5\n);
 
@@ -1326,7 +1365,7 @@ static struct i915_power_well chv_power_wells[] = {
 };
 
 static struct i915_power_well

[Intel-gfx] [PATCH 4/8] drm/i915/skl: Assert the requirements to enter or exit DC5.

2015-04-01 Thread Animesh Manna 
From: Suketu Shah suketu.j.s...@intel.com

Warn if the conditions to enter or exit DC5 are not satisfied such
as support for runtime PM, state of power well, CSR loading etc.

v2: Removed camelcase in functions and variables.

v3: Do some minimal check to assert if CSR program is not loaded.

v4:
1] Used an appropriate function lookup_power_well() to identify power well,
instead of using a magic number which can change in future.
2] Split the conditions further in assert_can_enable_DC5() and added more 
checks.
3] Removed all WARNs from assert_can_disable_DC5 as they were unnecessary and 
added two
   new ones.
4] Changed variable names as updated in earlier patches.

v5:
1] Change lookup_power_well function to take an int power well id.
2] Define a new intel_display_power_well_is_enabled helper function to check 
whether a
   particular power well is enabled.
3] Use CSR-related mutex in assert_csr_loaded function.

v6: Remove use of dc5_enabled variable as it's no longer needed.

v7:
1] Rebase to latest.
2] Move all DC5-related functions from intel_display.c to intel_runtime_pm.c.

v8: After adding dmc ver 1.0 support rebased on top of nightly. (Animesh)

Issue: VIZ-2819
Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com
Signed-off-by: Suketu Shah suketu.j.s...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Animesh Manna animesh.ma...@intel.com
---
 drivers/gpu/drm/i915/intel_drv.h|  2 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 61 ++---
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 39cb2dc..9aae624 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1232,6 +1232,8 @@ void intel_power_domains_fini(struct drm_i915_private *);
 void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
 
+bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
+   int power_well_id);
 bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain);
 bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8b917e2..f62d42b 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -335,12 +335,52 @@ static void gen9_set_dc_state_debugmask_memory_up(
}
 }
 
-static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
+static void assert_csr_loaded(struct drm_i915_private *dev_priv)
+{
+   mutex_lock(dev_priv-csr_lock);
+
+   WARN(!dev_priv-csr.loaded, CSR is not loaded.\n);
+   WARN(!I915_READ(CSR_PROGRAM_BASE),
+   CSR program storage start is NULL\n);
+   WARN(!I915_READ(CSR_SSP_BASE), CSR SSP Base Not fine\n);
+   WARN(!I915_READ(CSR_HTP_SKL), CSR HTP Not fine\n);
+
+   mutex_unlock(dev_priv-csr_lock);
+}
+
+static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
 {
struct drm_device *dev = dev_priv-dev;
+   bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
+   SKL_DISP_PW_2);
+
+   WARN(!IS_SKYLAKE(dev), Platform doesn't support DC5.\n);
+   WARN(!HAS_RUNTIME_PM(dev), Runtime PM not enabled.\n);
+   WARN(pg2_enabled, PG2 not disabled to enable DC5.\n);
+
+   WARN((I915_READ(DC_STATE_EN)  DC_STATE_EN_UPTO_DC5),
+   DC5 already programmed to be enabled.\n);
+   WARN(dev_priv-pm.suspended,
+   DC5 cannot be enabled, if platform is runtime-suspended.\n);
+
+   assert_csr_loaded(dev_priv);
+}
+
+static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
+{
+   bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
+   SKL_DISP_PW_2);
+
+   WARN(!pg2_enabled, PG2 not enabled to disable DC5.\n);
+   WARN(dev_priv-pm.suspended,
+   Disabling of DC5 while platform is runtime-suspended should 
never happen.\n);
+}
+
+static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
+{
uint32_t val;
 
-   WARN_ON(!IS_GEN9(dev));
+   assert_can_enable_dc5(dev_priv);
 
DRM_DEBUG_KMS(Enabling DC5\n);
 
@@ -355,10 +395,9 @@ static void gen9_enable_dc5(struct drm_i915_private 
*dev_priv)
 
 static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
 {
-   struct drm_device *dev = dev_priv-dev;
uint32_t val;
 
-   WARN_ON(!IS_GEN9(dev));
+   assert_can_disable_dc5(dev_priv);
 
DRM_DEBUG_KMS(Disabling DC5\n);
 
@@ -1326,7 +1365,7 @@ static struct i915_power_well chv_power_wells[] = {
 };
 
 static struct i915_power_well