Re: [Intel-gfx] [PATCH 6/8] drm/i915/skl: Add DC6 Trigger sequence.

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
 
 Add triggers for DC6 as per details provided in skl_enable_dc6
 and skl_disable_dc6 implementations.
 
 Also Call POSTING_READ for every write to a register to ensure
 it is written to immediately
 
 v1: Remove POSTING_READ and intel_prepare_ddi calls as they've been added in 
 previous patches.
 
 v2:
 1] Remove check for backlight disabled as it should be the case by that time.
 2] Mark DC5 as disabled when enabling DC6.
 3] Return from DC5-disabling function early if DC5 is already be disabled 
 which can happen
due to DC6-enabling earlier.
 3] Ensure CSR firmware is loaded after resume from DC6 as corresponding 
 memory contents won't
be retained after runtime-suspend.
 4] Ensure that CSR isn't identified as loaded before CSR-loading program is 
 called during
runtime-resume.
 
 v3: Rebase to latest
 Modified as per review comments from Imre and after discussion with Art:
 1] DC6 should be preferably enabled when PG2 is disabled by SW as the check 
 for PG1 being
disabled is taken of by HW to enter DC6, and disabled when PG2 is enabled 
 respectively.
This helps save more power, especially in the case when display is 
 disabled but GT is
enabled. Accordingly, replacing DC5 trigger sequence with DC6 for SKL.
 2] DC6 could be enabled from intel_runtime_suspend() function, if DC5 is 
 already enabled.
 3] Move CSR-load-status setting code from intel_runtime_suspend function to a 
 new function.
 
 v4:
 1] Enable/disable DC6 only when toggling the power-well using a newly defined 
 macro ENABLE_DC6.
 
 v5:
 1] Load CSR on system resume too as firmware may be lost on system suspend 
 preventing
enabling DC5, DC6.
 2] DDI buffers shouldn't be programmed during driver-load/resume as it's 
 already done
during modeset initialization then and also that the encoder list is still 
 uninitialized by
then. Therefore, call intel_prepare_ddi function right after disabling DC6 
 but outside
skl_disable_dc6 function and not during driver-load/resume.
 
 v6:
 1] Rebase to latest.
 2] Move SKL_ENABLE_DC6 macro definition from intel_display.c to 
 intel_runtime_pm.c.
 
 v7:
 1) Refactored the code for removing the warning got from checkpatch.
 2) 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/i915_drv.c | 29 ++
  drivers/gpu/drm/i915/i915_drv.h |  1 +
  drivers/gpu/drm/i915/intel_runtime_pm.c | 98 
 +
  3 files changed, 81 insertions(+), 47 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 489caa6..352c7702 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -795,6 +795,8 @@ static int i915_drm_resume_early(struct drm_device *dev)
  
   if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
   hsw_disable_pc8(dev_priv);
 + else if (IS_SKYLAKE(dev_priv))
 + ret = skl_resume_prepare(dev_priv);
  
   intel_uncore_sanitize(dev);
   intel_power_domains_init_hw(dev_priv);
 @@ -1009,6 +1011,19 @@ static int i915_pm_resume(struct device *dev)
   return i915_drm_resume(drm_dev);
  }
  
 +static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 +{
 + /* Enabling DC6 is not a hard requirement to enter runtime D3 */
 +
 + /*
 +  * This is to ensure that CSR isn't identified as loaded before
 +  * CSR-loading program is called during runtime-resume.
 +  */
 + intel_csr_load_status_set(dev_priv, false);
 +
 + return 0;
 +}
 +
  static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
  {
   hsw_enable_pc8(dev_priv);
 @@ -1016,6 +1031,16 @@ static int hsw_suspend_complete(struct 
 drm_i915_private *dev_priv)
   return 0;
  }
  
 +int skl_resume_prepare(struct drm_i915_private *dev_priv)
 +{
 + struct drm_device *dev = dev_priv-dev;
 +
 + if (!intel_csr_load_status_get(dev_priv))

No need to check the above, it's guaranteed the firmware is not loaded
at this point.

 + intel_csr_load_program(dev);
 +
 + return 0;
 +}
 +
  /*
   * Save all Gunit registers that may be lost after a D3 and a subsequent
   * S0i[R123] transition. The list of registers needing a save/restore is
 @@ -1484,6 +1509,8 @@ static int intel_runtime_resume(struct device *device)
  
   if (IS_GEN6(dev_priv))
   intel_init_pch_refclk(dev);
 + else if (IS_SKYLAKE(dev))
 + ret = skl_resume_prepare(dev_priv);
   else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
   hsw_disable_pc8(dev_priv);
   else if (IS_VALLEYVIEW(dev_priv))
 @@ 

[Intel-gfx] [PATCH 6/8] drm/i915/skl: Add DC6 Trigger sequence.

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

Add triggers for DC6 as per details provided in skl_enable_dc6
and skl_disable_dc6 implementations.

Also Call POSTING_READ for every write to a register to ensure
it is written to immediately

v1: Remove POSTING_READ and intel_prepare_ddi calls as they've been added in 
previous patches.

v2:
1] Remove check for backlight disabled as it should be the case by that time.
2] Mark DC5 as disabled when enabling DC6.
3] Return from DC5-disabling function early if DC5 is already be disabled which 
can happen
   due to DC6-enabling earlier.
3] Ensure CSR firmware is loaded after resume from DC6 as corresponding memory 
contents won't
   be retained after runtime-suspend.
4] Ensure that CSR isn't identified as loaded before CSR-loading program is 
called during
   runtime-resume.

v3: Rebase to latest
Modified as per review comments from Imre and after discussion with Art:
1] DC6 should be preferably enabled when PG2 is disabled by SW as the check for 
PG1 being
   disabled is taken of by HW to enter DC6, and disabled when PG2 is enabled 
respectively.
   This helps save more power, especially in the case when display is disabled 
but GT is
   enabled. Accordingly, replacing DC5 trigger sequence with DC6 for SKL.
2] DC6 could be enabled from intel_runtime_suspend() function, if DC5 is 
already enabled.
3] Move CSR-load-status setting code from intel_runtime_suspend function to a 
new function.

v4:
1] Enable/disable DC6 only when toggling the power-well using a newly defined 
macro ENABLE_DC6.

v5:
1] Load CSR on system resume too as firmware may be lost on system suspend 
preventing
   enabling DC5, DC6.
2] DDI buffers shouldn't be programmed during driver-load/resume as it's 
already done
   during modeset initialization then and also that the encoder list is still 
uninitialized by
   then. Therefore, call intel_prepare_ddi function right after disabling DC6 
but outside
   skl_disable_dc6 function and not during driver-load/resume.

v6:
1] Rebase to latest.
2] Move SKL_ENABLE_DC6 macro definition from intel_display.c to 
intel_runtime_pm.c.

v7:
1) Refactored the code for removing the warning got from checkpatch.
2) 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/i915_drv.c | 29 ++
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 98 +
 3 files changed, 81 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 489caa6..352c7702 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -795,6 +795,8 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
hsw_disable_pc8(dev_priv);
+   else if (IS_SKYLAKE(dev_priv))
+   ret = skl_resume_prepare(dev_priv);
 
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -1009,6 +1011,19 @@ static int i915_pm_resume(struct device *dev)
return i915_drm_resume(drm_dev);
 }
 
+static int skl_suspend_complete(struct drm_i915_private *dev_priv)
+{
+   /* Enabling DC6 is not a hard requirement to enter runtime D3 */
+
+   /*
+* This is to ensure that CSR isn't identified as loaded before
+* CSR-loading program is called during runtime-resume.
+*/
+   intel_csr_load_status_set(dev_priv, false);
+
+   return 0;
+}
+
 static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
 {
hsw_enable_pc8(dev_priv);
@@ -1016,6 +1031,16 @@ static int hsw_suspend_complete(struct drm_i915_private 
*dev_priv)
return 0;
 }
 
+int skl_resume_prepare(struct drm_i915_private *dev_priv)
+{
+   struct drm_device *dev = dev_priv-dev;
+
+   if (!intel_csr_load_status_get(dev_priv))
+   intel_csr_load_program(dev);
+
+   return 0;
+}
+
 /*
  * Save all Gunit registers that may be lost after a D3 and a subsequent
  * S0i[R123] transition. The list of registers needing a save/restore is
@@ -1484,6 +1509,8 @@ static int intel_runtime_resume(struct device *device)
 
if (IS_GEN6(dev_priv))
intel_init_pch_refclk(dev);
+   else if (IS_SKYLAKE(dev))
+   ret = skl_resume_prepare(dev_priv);
else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
hsw_disable_pc8(dev_priv);
else if (IS_VALLEYVIEW(dev_priv))
@@ -1516,6 +1543,8 @@ static int intel_suspend_complete(struct drm_i915_private 
*dev_priv)
struct drm_device *dev = dev_priv-dev;
int ret;
 
+   if (IS_SKYLAKE(dev))
+   ret = 

[Intel-gfx] [PATCH 6/8] drm/i915/skl: Add DC6 Trigger sequence.

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

Add triggers for DC6 as per details provided in skl_enable_dc6
and skl_disable_dc6 implementations.

Also Call POSTING_READ for every write to a register to ensure
it is written to immediately

v1: Remove POSTING_READ and intel_prepare_ddi calls as they've been added in 
previous patches.

v2:
1] Remove check for backlight disabled as it should be the case by that time.
2] Mark DC5 as disabled when enabling DC6.
3] Return from DC5-disabling function early if DC5 is already be disabled which 
can happen
   due to DC6-enabling earlier.
3] Ensure CSR firmware is loaded after resume from DC6 as corresponding memory 
contents won't
   be retained after runtime-suspend.
4] Ensure that CSR isn't identified as loaded before CSR-loading program is 
called during
   runtime-resume.

v3: Rebase to latest
Modified as per review comments from Imre and after discussion with Art:
1] DC6 should be preferably enabled when PG2 is disabled by SW as the check for 
PG1 being
   disabled is taken of by HW to enter DC6, and disabled when PG2 is enabled 
respectively.
   This helps save more power, especially in the case when display is disabled 
but GT is
   enabled. Accordingly, replacing DC5 trigger sequence with DC6 for SKL.
2] DC6 could be enabled from intel_runtime_suspend() function, if DC5 is 
already enabled.
3] Move CSR-load-status setting code from intel_runtime_suspend function to a 
new function.

v4:
1] Enable/disable DC6 only when toggling the power-well using a newly defined 
macro ENABLE_DC6.

v5:
1] Load CSR on system resume too as firmware may be lost on system suspend 
preventing
   enabling DC5, DC6.
2] DDI buffers shouldn't be programmed during driver-load/resume as it's 
already done
   during modeset initialization then and also that the encoder list is still 
uninitialized by
   then. Therefore, call intel_prepare_ddi function right after disabling DC6 
but outside
   skl_disable_dc6 function and not during driver-load/resume.

v6:
1] Rebase to latest.
2] Move SKL_ENABLE_DC6 macro definition from intel_display.c to 
intel_runtime_pm.c.

v7:
1) Refactored the code for removing the warning got from checkpatch.
2) 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/i915_drv.c | 29 ++
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 98 +
 3 files changed, 81 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 489caa6..352c7702 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -795,6 +795,8 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
hsw_disable_pc8(dev_priv);
+   else if (IS_SKYLAKE(dev_priv))
+   ret = skl_resume_prepare(dev_priv);
 
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -1009,6 +1011,19 @@ static int i915_pm_resume(struct device *dev)
return i915_drm_resume(drm_dev);
 }
 
+static int skl_suspend_complete(struct drm_i915_private *dev_priv)
+{
+   /* Enabling DC6 is not a hard requirement to enter runtime D3 */
+
+   /*
+* This is to ensure that CSR isn't identified as loaded before
+* CSR-loading program is called during runtime-resume.
+*/
+   intel_csr_load_status_set(dev_priv, false);
+
+   return 0;
+}
+
 static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
 {
hsw_enable_pc8(dev_priv);
@@ -1016,6 +1031,16 @@ static int hsw_suspend_complete(struct drm_i915_private 
*dev_priv)
return 0;
 }
 
+int skl_resume_prepare(struct drm_i915_private *dev_priv)
+{
+   struct drm_device *dev = dev_priv-dev;
+
+   if (!intel_csr_load_status_get(dev_priv))
+   intel_csr_load_program(dev);
+
+   return 0;
+}
+
 /*
  * Save all Gunit registers that may be lost after a D3 and a subsequent
  * S0i[R123] transition. The list of registers needing a save/restore is
@@ -1484,6 +1509,8 @@ static int intel_runtime_resume(struct device *device)
 
if (IS_GEN6(dev_priv))
intel_init_pch_refclk(dev);
+   else if (IS_SKYLAKE(dev))
+   ret = skl_resume_prepare(dev_priv);
else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
hsw_disable_pc8(dev_priv);
else if (IS_VALLEYVIEW(dev_priv))
@@ -1516,6 +1543,8 @@ static int intel_suspend_complete(struct drm_i915_private 
*dev_priv)
struct drm_device *dev = dev_priv-dev;
int ret;
 
+   if (IS_SKYLAKE(dev))
+   ret =