diff mbox

[15/18] drm/i915/skl: Making DC6 entry is the last call in suspend flow.

Message ID 1437850839-16782-16-git-send-email-animesh.manna@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Manna, Animesh July 25, 2015, 7 p.m. UTC 
Mmio register access after dc6/dc5 entry is causing the
system hang, so enabling dc6 as the last call in suspend flow.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  6 ++++++
 drivers/gpu/drm/i915/intel_drv.h        |  2 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++------------
 3 files changed, 15 insertions(+), 12 deletions(-)

Comments

Kamath, Sunil July 30, 2015, 7:06 a.m. UTC | #1 
On Sunday 26 July 2015 12:30 AM, Animesh Manna wrote:
> Mmio register access after dc6/dc5 entry is causing the
> system hang, so enabling dc6 as the last call in suspend flow.
>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c         |  6 ++++++
>   drivers/gpu/drm/i915/intel_drv.h        |  2 ++
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++------------
>   3 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ddf8a25..77b35fd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -995,6 +995,9 @@ static int i915_pm_resume(struct device *dev)
>   
>   static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>   {
> +	if (dev_priv->csr.dmc_payload)
> +		skl_enable_dc6(dev_priv);
> +
>   	skl_uninit_cdclk(dev_priv);

This is really right change.
With this we will go back to our original design.
Deepest possible display state will be triggered by respective suspend 
complete.
for example in BXT we trigger DC9 from bxt_suspend_complete and works 
fine with rpm integrated.
Same case here for SKL for DC6 which uses DMC.

Right bug fix.

Reviewed-by: A.Sunil Kamath <sunil.kamath@intel.com>

>   
>   	return 0;
> @@ -1041,6 +1044,9 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
>   
>   static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>   {
> +	if (dev_priv->csr.dmc_payload)
> +		skl_disable_dc6(dev_priv);
> +
>   	skl_init_cdclk(dev_priv);
>   	intel_csr_load_program(dev_priv);
>   
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 644286f..0d13f50 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1117,6 +1117,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>   void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>   void skl_init_cdclk(struct drm_i915_private *dev_priv);
>   void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
> +void skl_enable_dc6(struct drm_i915_private *dev_priv);
> +void skl_disable_dc6(struct drm_i915_private *dev_priv);
>   void intel_dp_get_m_n(struct intel_crtc *crtc,
>   		      struct intel_crtc_state *pipe_config);
>   void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index a5059e8..ddae00e 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -540,7 +540,7 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
>   		"DC6 already programmed to be disabled.\n");
>   }
>   
> -static void skl_enable_dc6(struct drm_i915_private *dev_priv)
> +void skl_enable_dc6(struct drm_i915_private *dev_priv)
>   {
>   	uint32_t val;
>   
> @@ -557,7 +557,7 @@ static void skl_enable_dc6(struct drm_i915_private *dev_priv)
>   	POSTING_READ(DC_STATE_EN);
>   }
>   
> -static void skl_disable_dc6(struct drm_i915_private *dev_priv)
> +void skl_disable_dc6(struct drm_i915_private *dev_priv)
>   {
>   	uint32_t val;
>   
> @@ -618,10 +618,10 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>   				!I915_READ(HSW_PWR_WELL_BIOS),
>   				"Invalid for power well status to be enabled, unless done by the BIOS, \
>   				when request is to disable!\n");
> -			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
> -				power_well->data == SKL_DISP_PW_2) {
> +			if (power_well->data == SKL_DISP_PW_2) {
> +				if (GEN9_ENABLE_DC5(dev))
> +					gen9_disable_dc5(dev_priv);
>   				if (SKL_ENABLE_DC6(dev)) {
> -					skl_disable_dc6(dev_priv);
>   					/*
>   					 * DDI buffer programming unnecessary during driver-load/resume
>   					 * as it's already done during modeset initialization then.
> @@ -629,8 +629,6 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>   					 */
>   					if (!dev_priv->power_domains.initializing)
>   						intel_prepare_ddi(dev);
> -				} else {
> -					gen9_disable_dc5(dev_priv);
>   				}
>   			}
>   			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
> @@ -650,12 +648,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>   			POSTING_READ(HSW_PWR_WELL_DRIVER);
>   			DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>   
> -			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
> -				power_well->data == SKL_DISP_PW_2) {
> +			if (power_well->data == SKL_DISP_PW_2) {
>   				flush_work(&dev_priv->csr.work);
> -				if (SKL_ENABLE_DC6(dev))
> -					skl_enable_dc6(dev_priv);
> -				else
> +				if (GEN9_ENABLE_DC5(dev))
>   					gen9_enable_dc5(dev_priv);
>   			}
>   		}
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ddf8a25..77b35fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -995,6 +995,9 @@  static int i915_pm_resume(struct device *dev)
 
 static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
+	if (dev_priv->csr.dmc_payload)
+		skl_enable_dc6(dev_priv);
+
 	skl_uninit_cdclk(dev_priv);
 
 	return 0;
@@ -1041,6 +1044,9 @@  static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
 
 static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
+	if (dev_priv->csr.dmc_payload)
+		skl_disable_dc6(dev_priv);
+
 	skl_init_cdclk(dev_priv);
 	intel_csr_load_program(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 644286f..0d13f50 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1117,6 +1117,8 @@  void bxt_enable_dc9(struct drm_i915_private *dev_priv);
 void bxt_disable_dc9(struct drm_i915_private *dev_priv);
 void skl_init_cdclk(struct drm_i915_private *dev_priv);
 void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
+void skl_enable_dc6(struct drm_i915_private *dev_priv);
+void skl_disable_dc6(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_state *pipe_config);
 void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index a5059e8..ddae00e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -540,7 +540,7 @@  static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
 		"DC6 already programmed to be disabled.\n");
 }
 
-static void skl_enable_dc6(struct drm_i915_private *dev_priv)
+void skl_enable_dc6(struct drm_i915_private *dev_priv)
 {
 	uint32_t val;
 
@@ -557,7 +557,7 @@  static void skl_enable_dc6(struct drm_i915_private *dev_priv)
 	POSTING_READ(DC_STATE_EN);
 }
 
-static void skl_disable_dc6(struct drm_i915_private *dev_priv)
+void skl_disable_dc6(struct drm_i915_private *dev_priv)
 {
 	uint32_t val;
 
@@ -618,10 +618,10 @@  static void skl_set_power_well(struct drm_i915_private *dev_priv,
 				!I915_READ(HSW_PWR_WELL_BIOS),
 				"Invalid for power well status to be enabled, unless done by the BIOS, \
 				when request is to disable!\n");
-			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
-				power_well->data == SKL_DISP_PW_2) {
+			if (power_well->data == SKL_DISP_PW_2) {
+				if (GEN9_ENABLE_DC5(dev))
+					gen9_disable_dc5(dev_priv);
 				if (SKL_ENABLE_DC6(dev)) {
-					skl_disable_dc6(dev_priv);
 					/*
 					 * DDI buffer programming unnecessary during driver-load/resume
 					 * as it's already done during modeset initialization then.
@@ -629,8 +629,6 @@  static void skl_set_power_well(struct drm_i915_private *dev_priv,
 					 */
 					if (!dev_priv->power_domains.initializing)
 						intel_prepare_ddi(dev);
-				} else {
-					gen9_disable_dc5(dev_priv);
 				}
 			}
 			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
@@ -650,12 +648,9 @@  static void skl_set_power_well(struct drm_i915_private *dev_priv,
 			POSTING_READ(HSW_PWR_WELL_DRIVER);
 			DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
 
-			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
-				power_well->data == SKL_DISP_PW_2) {
+			if (power_well->data == SKL_DISP_PW_2) {
 				flush_work(&dev_priv->csr.work);
-				if (SKL_ENABLE_DC6(dev))
-					skl_enable_dc6(dev_priv);
-				else
+				if (GEN9_ENABLE_DC5(dev))
 					gen9_enable_dc5(dev_priv);
 			}
 		}