[PATCH 0/4] drm/i915/bxt: Fixes for runtime and system suspend/resume

[PATCH 0/4] drm/i915/bxt: Fixes for runtime and system suspend/resume

[Imre Deak]

These are fixes for issues I noticed during further testing of runtime
and system suspend on BXT. One of them was a false positive state
verification error for DC9 and the other is delaying DC5 enabling
unnecessarily.

CC: Matt Roper <matthew.d.roper@intel.com>

Imre Deak (4):
  drm/i915: Uninline intel_suspend_complete
  drm/i915/bxt: Don't uninit/init display core twice during system
    suspend/resume
  drm/i915/bxt: Sanitize DC state tracking during system resume
  drm/i915/bxt: Enable DC5 during runtime resume

 drivers/gpu/drm/i915/i915_drv.c         | 87 +++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h        |  2 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 27 ++++++++--
 3 files changed, 58 insertions(+), 58 deletions(-)

-- 
2.5.0

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

[PATCH 1/4] drm/i915: Uninline intel_suspend_complete

[Imre Deak]

Initially we thought that the platform specific suspend/resume sequences
can be shared between the runtime and system suspend/resume handlers.
This turned out to be not true, we have quite a few differences on most
of the platforms. This was realized already earlier by Paulo who
uninlined the platform specific resume_prepare handlers. We have the
same problem with the corresponding suspend_complete handlers, there are
platform differences that make it unfeasible to share the code between
the runtime and system suspend paths. Also now we call functions that
need to be paired like hsw_enable_pc8()/hsw_disable_pc8() from different
levels of the call stack, which is confusing. Fix this by uninlining the
suspend_complete handlers too.

This is also needed by the next patch that removes a redundant
uninit/init call during system suspend/resume on BXT.

No functional change.

CC: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 83 ++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9a016f1..ea9b3fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -567,10 +567,9 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 	drm_modeset_unlock_all(dev);
 }
 
-static int intel_suspend_complete(struct drm_i915_private *dev_priv);
 static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
 			      bool rpm_resume);
-static int bxt_resume_prepare(struct drm_i915_private *dev_priv);
+static int vlv_suspend_complete(struct drm_i915_private *dev_priv);
 
 static bool suspend_to_idle(struct drm_i915_private *dev_priv)
 {
@@ -668,7 +667,14 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
 	if (!fw_csr)
 		intel_power_domains_suspend(dev_priv);
 
-	ret = intel_suspend_complete(dev_priv);
+	ret = 0;
+	if (IS_BROXTON(dev_priv)) {
+		bxt_display_core_uninit(dev_priv);
+		bxt_enable_dc9(dev_priv);
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		hsw_enable_pc8(dev_priv);
+	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		ret = vlv_suspend_complete(dev_priv);
 
 	if (ret) {
 		DRM_ERROR("Suspend complete failed: %d\n", ret);
@@ -862,9 +868,10 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	intel_uncore_early_sanitize(dev, true);
 
-	if (IS_BROXTON(dev))
-		ret = bxt_resume_prepare(dev_priv);
-	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	if (IS_BROXTON(dev)) {
+		bxt_disable_dc9(dev_priv);
+		bxt_display_core_init(dev_priv, true);
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hsw_disable_pc8(dev_priv);
 
 	intel_uncore_sanitize(dev);
@@ -1102,29 +1109,6 @@ static int i915_pm_resume(struct device *dev)
 	return i915_drm_resume(drm_dev);
 }
 
-static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
-{
-	hsw_enable_pc8(dev_priv);
-
-	return 0;
-}
-
-static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
-{
-	bxt_display_core_uninit(dev_priv);
-	bxt_enable_dc9(dev_priv);
-
-	return 0;
-}
-
-static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
-{
-	bxt_disable_dc9(dev_priv);
-	bxt_display_core_init(dev_priv, true);
-
-	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
@@ -1530,7 +1514,16 @@ static int intel_runtime_suspend(struct device *device)
 	intel_suspend_gt_powersave(dev);
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
-	ret = intel_suspend_complete(dev_priv);
+	ret = 0;
+	if (IS_BROXTON(dev_priv)) {
+		bxt_display_core_uninit(dev_priv);
+		bxt_enable_dc9(dev_priv);
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		hsw_enable_pc8(dev_priv);
+	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		ret = vlv_suspend_complete(dev_priv);
+	}
+
 	if (ret) {
 		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
 		intel_runtime_pm_enable_interrupts(dev_priv);
@@ -1604,12 +1597,14 @@ static int intel_runtime_resume(struct device *device)
 	if (IS_GEN6(dev_priv))
 		intel_init_pch_refclk(dev);
 
-	if (IS_BROXTON(dev))
-		ret = bxt_resume_prepare(dev_priv);
-	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	if (IS_BROXTON(dev)) {
+		bxt_disable_dc9(dev_priv);
+		bxt_display_core_init(dev_priv, true);
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		hsw_disable_pc8(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		ret = vlv_resume_prepare(dev_priv, true);
+	}
 
 	/*
 	 * No point of rolling back things in case of an error, as the best
@@ -1640,26 +1635,6 @@ static int intel_runtime_resume(struct device *device)
 	return ret;
 }
 
-/*
- * This function implements common functionality of runtime and system
- * suspend sequence.
- */
-static int intel_suspend_complete(struct drm_i915_private *dev_priv)
-{
-	int ret;
-
-	if (IS_BROXTON(dev_priv))
-		ret = bxt_suspend_complete(dev_priv);
-	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		ret = hsw_suspend_complete(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		ret = vlv_suspend_complete(dev_priv);
-	else
-		ret = 0;
-
-	return ret;
-}
-
 static const struct dev_pm_ops i915_pm_ops = {
 	/*
 	 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
-- 
2.5.0

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

[PATCH 2/4] drm/i915/bxt: Don't uninit/init display core twice during system suspend/resume

[Imre Deak]

Atm, we run the BSpec display core uninit/init sequences twice during
system suspend/resume. While this shouldn't cause any problem, it's
redundant, so get rid of the duplicate call.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ea9b3fe..4dc2904 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -668,10 +668,9 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
 		intel_power_domains_suspend(dev_priv);
 
 	ret = 0;
-	if (IS_BROXTON(dev_priv)) {
-		bxt_display_core_uninit(dev_priv);
+	if (IS_BROXTON(dev_priv))
 		bxt_enable_dc9(dev_priv);
-	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hsw_enable_pc8(dev_priv);
 	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		ret = vlv_suspend_complete(dev_priv);
@@ -868,10 +867,9 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	intel_uncore_early_sanitize(dev, true);
 
-	if (IS_BROXTON(dev)) {
+	if (IS_BROXTON(dev))
 		bxt_disable_dc9(dev_priv);
-		bxt_display_core_init(dev_priv, true);
-	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hsw_disable_pc8(dev_priv);
 
 	intel_uncore_sanitize(dev);
-- 
2.5.0

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

[PATCH 3/4] drm/i915/bxt: Sanitize DC state tracking during system resume

[Imre Deak]

After suspend-to-ram or -disk we don't know what power state the display
HW will be, DC0 or DC9 are both possible states, so reset the software
DC state tracking in these cases. This gets rid of 'DC state mismatch'
error messages during resuming from ram or disk where we expected to be
in DC9 (as set by the suspend handler) but we are in DC0.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  7 +++++--
 drivers/gpu/drm/i915/intel_drv.h        |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++++++++++++---
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4dc2904..9f55631 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -867,10 +867,13 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	intel_uncore_early_sanitize(dev, true);
 
-	if (IS_BROXTON(dev))
+	if (IS_BROXTON(dev)) {
+		if (!dev_priv->suspended_to_idle)
+			gen9_sanitize_dc_state(dev_priv);
 		bxt_disable_dc9(dev_priv);
-	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		hsw_disable_pc8(dev_priv);
+	}
 
 	intel_uncore_sanitize(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index beed9e8..5464632 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1235,6 +1235,7 @@ bool broxton_cdclk_verify_state(struct drm_i915_private *dev_priv);
 void broxton_ddi_phy_init(struct drm_i915_private *dev_priv);
 void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv);
 void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv);
+void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv);
 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);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 06d14c4..329784e 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -492,10 +492,9 @@ static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
 			      state, rewrites);
 }
 
-static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
+static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
 {
-	uint32_t val;
-	uint32_t mask;
+	u32 mask;
 
 	mask = DC_STATE_EN_UPTO_DC5;
 	if (IS_BROXTON(dev_priv))
@@ -503,10 +502,30 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
 	else
 		mask |= DC_STATE_EN_UPTO_DC6;
 
+	return mask;
+}
+
+void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	val = I915_READ(DC_STATE_EN) & gen9_dc_mask(dev_priv);
+
+	DRM_DEBUG_KMS("Resetting DC state tracking from %02x to %02x\n",
+		      dev_priv->csr.dc_state, val);
+	dev_priv->csr.dc_state =val;
+}
+
+static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
+{
+	uint32_t val;
+	uint32_t mask;
+
 	if (WARN_ON_ONCE(state & ~dev_priv->csr.allowed_dc_mask))
 		state &= dev_priv->csr.allowed_dc_mask;
 
 	val = I915_READ(DC_STATE_EN);
+	mask = gen9_dc_mask(dev_priv);
 	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n",
 		      val & mask, state);
 
-- 
2.5.0

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

[PATCH 4/4] drm/i915/bxt: Enable DC5 during runtime resume

[Imre Deak]

Right after runtime resume we know that we can re-enable DC5, since we
just disabled DC9 and power well 2 is disabled. So enable DC5 explicitly
instead of delaying this until the next time we disable power well 2.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 3 +++
 drivers/gpu/drm/i915/intel_drv.h        | 1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9f55631..d37c0a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1601,6 +1601,9 @@ static int intel_runtime_resume(struct device *device)
 	if (IS_BROXTON(dev)) {
 		bxt_disable_dc9(dev_priv);
 		bxt_display_core_init(dev_priv, true);
+		if (dev_priv->csr.dmc_payload &&
+		    (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5))
+			gen9_enable_dc5(dev_priv);
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		hsw_disable_pc8(dev_priv);
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5464632..b9f1304 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1238,6 +1238,7 @@ void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv);
 void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv);
 void bxt_enable_dc9(struct drm_i915_private *dev_priv);
 void bxt_disable_dc9(struct drm_i915_private *dev_priv);
+void gen9_enable_dc5(struct drm_i915_private *dev_priv);
 void skl_init_cdclk(struct drm_i915_private *dev_priv);
 int skl_sanitize_cdclk(struct drm_i915_private *dev_priv);
 void skl_uninit_cdclk(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 329784e..3036962 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -582,7 +582,7 @@ static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
 	assert_csr_loaded(dev_priv);
 }
 
-static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
+void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 {
 	assert_can_enable_dc5(dev_priv);
 
-- 
2.5.0

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

Re: [PATCH 1/4] drm/i915: Uninline intel_suspend_complete

[Bob Paauwe]

On Wed, 20 Apr 2016 20:27:54 +0300
Imre Deak <imre.deak@intel.com> wrote:

> Initially we thought that the platform specific suspend/resume sequences
> can be shared between the runtime and system suspend/resume handlers.
> This turned out to be not true, we have quite a few differences on most
> of the platforms. This was realized already earlier by Paulo who
> uninlined the platform specific resume_prepare handlers. We have the
> same problem with the corresponding suspend_complete handlers, there are
> platform differences that make it unfeasible to share the code between
> the runtime and system suspend paths. Also now we call functions that
> need to be paired like hsw_enable_pc8()/hsw_disable_pc8() from different
> levels of the call stack, which is confusing. Fix this by uninlining the
> suspend_complete handlers too.
> 
> This is also needed by the next patch that removes a redundant
> uninit/init call during system suspend/resume on BXT.
> 
> No functional change.

Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

> 
> CC: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 83 ++++++++++++++---------------------------
>  1 file changed, 29 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9a016f1..ea9b3fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -567,10 +567,9 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  	drm_modeset_unlock_all(dev);
>  }
>  
> -static int intel_suspend_complete(struct drm_i915_private *dev_priv);
>  static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
>  			      bool rpm_resume);
> -static int bxt_resume_prepare(struct drm_i915_private *dev_priv);
> +static int vlv_suspend_complete(struct drm_i915_private *dev_priv);
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
>  {
> @@ -668,7 +667,14 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
>  	if (!fw_csr)
>  		intel_power_domains_suspend(dev_priv);
>  
> -	ret = intel_suspend_complete(dev_priv);
> +	ret = 0;
> +	if (IS_BROXTON(dev_priv)) {
> +		bxt_display_core_uninit(dev_priv);
> +		bxt_enable_dc9(dev_priv);
> +	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +		hsw_enable_pc8(dev_priv);
> +	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +		ret = vlv_suspend_complete(dev_priv);
>  
>  	if (ret) {
>  		DRM_ERROR("Suspend complete failed: %d\n", ret);
> @@ -862,9 +868,10 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  
>  	intel_uncore_early_sanitize(dev, true);
>  
> -	if (IS_BROXTON(dev))
> -		ret = bxt_resume_prepare(dev_priv);
> -	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +	if (IS_BROXTON(dev)) {
> +		bxt_disable_dc9(dev_priv);
> +		bxt_display_core_init(dev_priv, true);
> +	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		hsw_disable_pc8(dev_priv);
>  
>  	intel_uncore_sanitize(dev);
> @@ -1102,29 +1109,6 @@ static int i915_pm_resume(struct device *dev)
>  	return i915_drm_resume(drm_dev);
>  }
>  
> -static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
> -{
> -	hsw_enable_pc8(dev_priv);
> -
> -	return 0;
> -}
> -
> -static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
> -{
> -	bxt_display_core_uninit(dev_priv);
> -	bxt_enable_dc9(dev_priv);
> -
> -	return 0;
> -}
> -
> -static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
> -{
> -	bxt_disable_dc9(dev_priv);
> -	bxt_display_core_init(dev_priv, true);
> -
> -	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
> @@ -1530,7 +1514,16 @@ static int intel_runtime_suspend(struct device *device)
>  	intel_suspend_gt_powersave(dev);
>  	intel_runtime_pm_disable_interrupts(dev_priv);
>  
> -	ret = intel_suspend_complete(dev_priv);
> +	ret = 0;
> +	if (IS_BROXTON(dev_priv)) {
> +		bxt_display_core_uninit(dev_priv);
> +		bxt_enable_dc9(dev_priv);
> +	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> +		hsw_enable_pc8(dev_priv);
> +	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		ret = vlv_suspend_complete(dev_priv);
> +	}
> +
>  	if (ret) {
>  		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
>  		intel_runtime_pm_enable_interrupts(dev_priv);
> @@ -1604,12 +1597,14 @@ static int intel_runtime_resume(struct device *device)
>  	if (IS_GEN6(dev_priv))
>  		intel_init_pch_refclk(dev);
>  
> -	if (IS_BROXTON(dev))
> -		ret = bxt_resume_prepare(dev_priv);
> -	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +	if (IS_BROXTON(dev)) {
> +		bxt_disable_dc9(dev_priv);
> +		bxt_display_core_init(dev_priv, true);
> +	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		hsw_disable_pc8(dev_priv);
> -	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		ret = vlv_resume_prepare(dev_priv, true);
> +	}
>  
>  	/*
>  	 * No point of rolling back things in case of an error, as the best
> @@ -1640,26 +1635,6 @@ static int intel_runtime_resume(struct device *device)
>  	return ret;
>  }
>  
> -/*
> - * This function implements common functionality of runtime and system
> - * suspend sequence.
> - */
> -static int intel_suspend_complete(struct drm_i915_private *dev_priv)
> -{
> -	int ret;
> -
> -	if (IS_BROXTON(dev_priv))
> -		ret = bxt_suspend_complete(dev_priv);
> -	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -		ret = hsw_suspend_complete(dev_priv);
> -	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		ret = vlv_suspend_complete(dev_priv);
> -	else
> -		ret = 0;
> -
> -	return ret;
> -}
> -
>  static const struct dev_pm_ops i915_pm_ops = {
>  	/*
>  	 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

Re: [PATCH 3/4] drm/i915/bxt: Sanitize DC state tracking during system resume

[Bob Paauwe]

On Wed, 20 Apr 2016 20:27:56 +0300
Imre Deak <imre.deak@intel.com> wrote:

> After suspend-to-ram or -disk we don't know what power state the display
> HW will be, DC0 or DC9 are both possible states, so reset the software
> DC state tracking in these cases. This gets rid of 'DC state mismatch'
> error messages during resuming from ram or disk where we expected to be
> in DC9 (as set by the suspend handler) but we are in DC0.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  7 +++++--
>  drivers/gpu/drm/i915/intel_drv.h        |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 25 ++++++++++++++++++++++---
>  3 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 4dc2904..9f55631 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -867,10 +867,13 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  
>  	intel_uncore_early_sanitize(dev, true);
>  
> -	if (IS_BROXTON(dev))
> +	if (IS_BROXTON(dev)) {
> +		if (!dev_priv->suspended_to_idle)
> +			gen9_sanitize_dc_state(dev_priv);
>  		bxt_disable_dc9(dev_priv);
> -	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		hsw_disable_pc8(dev_priv);
> +	}
>  
>  	intel_uncore_sanitize(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index beed9e8..5464632 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1235,6 +1235,7 @@ bool broxton_cdclk_verify_state(struct drm_i915_private *dev_priv);
>  void broxton_ddi_phy_init(struct drm_i915_private *dev_priv);
>  void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv);
>  void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv);
> +void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv);
>  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);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 06d14c4..329784e 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -492,10 +492,9 @@ static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
>  			      state, rewrites);
>  }
>  
> -static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
> +static u32 gen9_dc_mask(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -	uint32_t mask;
> +	u32 mask;
>  
>  	mask = DC_STATE_EN_UPTO_DC5;
>  	if (IS_BROXTON(dev_priv))
> @@ -503,10 +502,30 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
>  	else
>  		mask |= DC_STATE_EN_UPTO_DC6;
>  
> +	return mask;
> +}
> +
> +void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +
> +	val = I915_READ(DC_STATE_EN) & gen9_dc_mask(dev_priv);
> +
> +	DRM_DEBUG_KMS("Resetting DC state tracking from %02x to %02x\n",
> +		      dev_priv->csr.dc_state, val);
> +	dev_priv->csr.dc_state =val;

Just a nit, but missing white space after '='

With that, Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

> +}
> +
> +static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
> +{
> +	uint32_t val;
> +	uint32_t mask;
> +
>  	if (WARN_ON_ONCE(state & ~dev_priv->csr.allowed_dc_mask))
>  		state &= dev_priv->csr.allowed_dc_mask;
>  
>  	val = I915_READ(DC_STATE_EN);
> +	mask = gen9_dc_mask(dev_priv);
>  	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n",
>  		      val & mask, state);
>  



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

Re: [PATCH 4/4] drm/i915/bxt: Enable DC5 during runtime resume

[Bob Paauwe]

On Wed, 20 Apr 2016 20:27:57 +0300
Imre Deak <imre.deak@intel.com> wrote:

> Right after runtime resume we know that we can re-enable DC5, since we
> just disabled DC9 and power well 2 is disabled. So enable DC5 explicitly
> instead of delaying this until the next time we disable power well 2.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 3 +++
>  drivers/gpu/drm/i915/intel_drv.h        | 1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9f55631..d37c0a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1601,6 +1601,9 @@ static int intel_runtime_resume(struct device *device)
>  	if (IS_BROXTON(dev)) {
>  		bxt_disable_dc9(dev_priv);
>  		bxt_display_core_init(dev_priv, true);
> +		if (dev_priv->csr.dmc_payload &&
> +		    (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5))
> +			gen9_enable_dc5(dev_priv);
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		hsw_disable_pc8(dev_priv);
>  	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5464632..b9f1304 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1238,6 +1238,7 @@ void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv);
>  void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv);
>  void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> +void gen9_enable_dc5(struct drm_i915_private *dev_priv);
>  void skl_init_cdclk(struct drm_i915_private *dev_priv);
>  int skl_sanitize_cdclk(struct drm_i915_private *dev_priv);
>  void skl_uninit_cdclk(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 329784e..3036962 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -582,7 +582,7 @@ static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
>  	assert_csr_loaded(dev_priv);
>  }
>  
> -static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
> +void gen9_enable_dc5(struct drm_i915_private *dev_priv)
>  {
>  	assert_can_enable_dc5(dev_priv);
>  



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

Re: [PATCH 2/4] drm/i915/bxt: Don't uninit/init display core twice during system suspend/resume

[Bob Paauwe]

On Wed, 20 Apr 2016 20:27:55 +0300
Imre Deak <imre.deak@intel.com> wrote:

> Atm, we run the BSpec display core uninit/init sequences twice during
> system suspend/resume. While this shouldn't cause any problem, it's
> redundant, so get rid of the duplicate call.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ea9b3fe..4dc2904 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -668,10 +668,9 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
>  		intel_power_domains_suspend(dev_priv);
>  
>  	ret = 0;
> -	if (IS_BROXTON(dev_priv)) {
> -		bxt_display_core_uninit(dev_priv);
> +	if (IS_BROXTON(dev_priv))
>  		bxt_enable_dc9(dev_priv);
> -	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		hsw_enable_pc8(dev_priv);
>  	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		ret = vlv_suspend_complete(dev_priv);
> @@ -868,10 +867,9 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  
>  	intel_uncore_early_sanitize(dev, true);
>  
> -	if (IS_BROXTON(dev)) {
> +	if (IS_BROXTON(dev))
>  		bxt_disable_dc9(dev_priv);
> -		bxt_display_core_init(dev_priv, true);
> -	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		hsw_disable_pc8(dev_priv);
>  
>  	intel_uncore_sanitize(dev);



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

✓ Fi.CI.BAT: success for drm/i915/bxt: Fixes for runtime and system suspend/resume

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fixes for runtime and system suspend/resume
URL   : https://patchwork.freedesktop.org/series/6009/
State : success

== Summary ==

Series 6009v1 drm/i915/bxt: Fixes for runtime and system suspend/resume
http://patchwork.freedesktop.org/api/1.0/series/6009/revisions/1/mbox/


bdw-ultra        total:194  pass:170  dwarn:0   dfail:0   fail:1   skip:23 
bsw-nuc-2        total:193  pass:154  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:193  pass:154  dwarn:0   dfail:0   fail:1   skip:38 
hsw-brixbox      total:194  pass:170  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:194  pass:175  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:194  pass:136  dwarn:0   dfail:1   fail:0   skip:57 
ivb-t430s        total:194  pass:166  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:194  pass:168  dwarn:0   dfail:0   fail:1   skip:25 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:152  pass:115  dwarn:0   dfail:0   fail:0   skip:36 

Results at /archive/results/CI_IGT_test/Patchwork_1966/

9dabb0053b63bc32ab6ad5d13209d1e43395313f drm-intel-nightly: 2016y-04m-21d-09h-27m-12s UTC integration manifest
1969810 drm/i915/bxt: Enable DC5 during runtime resume
4e6ae40 drm/i915/bxt: Sanitize DC state tracking during system resume
5e7b5f6 drm/i915/bxt: Don't uninit/init display core twice during system suspend/resume
b5a5dbd drm/i915: Uninline intel_suspend_complete

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

✗ Fi.CI.BAT: failure for drm/i915/bxt: Fixes for runtime and system suspend/resume

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fixes for runtime and system suspend/resume
URL   : https://patchwork.freedesktop.org/series/6009/
State : failure

== Summary ==

Series 6009v1 drm/i915/bxt: Fixes for runtime and system suspend/resume
http://patchwork.freedesktop.org/api/1.0/series/6009/revisions/1/mbox/

Test gem_busy:
        Subgroup basic-blt:
                skip       -> PASS       (bsw-nuc-2)
Test gem_sync:
        Subgroup basic-bsd:
                pass       -> DMESG-FAIL (ilk-hp8440p)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (byt-nuc)

bdw-ultra        total:194  pass:170  dwarn:0   dfail:0   fail:1   skip:23 
bsw-nuc-2        total:193  pass:154  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:193  pass:154  dwarn:0   dfail:0   fail:1   skip:38 
hsw-brixbox      total:194  pass:170  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:194  pass:175  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:194  pass:136  dwarn:0   dfail:1   fail:0   skip:57 
ivb-t430s        total:194  pass:166  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:194  pass:168  dwarn:0   dfail:0   fail:1   skip:25 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:152  pass:115  dwarn:0   dfail:0   fail:0   skip:36 

Results at /archive/results/CI_IGT_test/Patchwork_1966/

9dabb0053b63bc32ab6ad5d13209d1e43395313f drm-intel-nightly: 2016y-04m-21d-09h-27m-12s UTC integration manifest
1969810 drm/i915/bxt: Enable DC5 during runtime resume
4e6ae40 drm/i915/bxt: Sanitize DC state tracking during system resume
5e7b5f6 drm/i915/bxt: Don't uninit/init display core twice during system suspend/resume
b5a5dbd drm/i915: Uninline intel_suspend_complete

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

Re: ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fixes for runtime and system suspend/resume

[Imre Deak]

On to, 2016-04-21 at 12:05 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/bxt: Fixes for runtime and system suspend/resume
> URL   : https://patchwork.freedesktop.org/series/6009/
> State : failure
> 
> == Summary ==

There was a previous email with 'success' for this same test run
(Patchwork_1966).

> Series 6009v1 drm/i915/bxt: Fixes for runtime and system
> suspend/resume
> http://patchwork.freedesktop.org/api/1.0/series/6009/revisions/1/mbox
> /
> 
> Test gem_busy:
>         Subgroup basic-blt:
>                 skip       -> PASS       (bsw-nuc-2)
> Test gem_sync:
>         Subgroup basic-bsd:
>                 pass       -> DMESG-FAIL (ilk-hp8440p)

GPU hang on BSD ring:
https://bugs.freedesktop.org/show_bug.cgi?id=94307

> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> FAIL       (byt-nuc)

(kms_flip:7340) CRITICAL: Test assertion failure function check_state, file kms_flip.c:692:
(kms_flip:7340) CRITICAL: Failed assertion: fabs((((double) diff.tv_usec) - usec_interflip) / usec_interflip) <= 0.005
(kms_flip:7340) CRITICAL: Last errno: 25, Inappropriate ioctl for device
(kms_flip:7340) CRITICAL: inter-vblank ts jitter: 0s, 183433usec
Subtest basic-flip-vs-wf_vblank failed.

Thanks for the review, I pushed the patchset to -dinq.

I fixed while applying the WS error you found in patch 3/4 and fixed my
uninline vs. inline confusion in the commit message of 1/4.

--Imre

> 
> bdw-
> ultra        total:194  pass:170  dwarn:0   dfail:0   fail:1   skip:2
> 3 
> bsw-nuc-
> 2        total:193  pass:154  dwarn:0   dfail:0   fail:0   skip:39 
> byt-
> nuc          total:193  pass:154  dwarn:0   dfail:0   fail:1   skip:3
> 8 
> hsw-
> brixbox      total:194  pass:170  dwarn:0   dfail:0   fail:0   skip:2
> 4 
> hsw-
> gt2          total:194  pass:175  dwarn:0   dfail:0   fail:0   skip:1
> 9 
> ilk-
> hp8440p      total:194  pass:136  dwarn:0   dfail:1   fail:0   skip:5
> 7 
> ivb-
> t430s        total:194  pass:166  dwarn:0   dfail:0   fail:0   skip:2
> 8 
> skl-i7k-
> 2        total:194  pass:168  dwarn:0   dfail:0   fail:1   skip:25 
> skl-
> nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:1
> 1 
> snb-
> dellxps      total:152  pass:115  dwarn:0   dfail:0   fail:0   skip:3
> 6 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1966/
> 
> 9dabb0053b63bc32ab6ad5d13209d1e43395313f drm-intel-nightly: 2016y-
> 04m-21d-09h-27m-12s UTC integration manifest
> 1969810 drm/i915/bxt: Enable DC5 during runtime resume
> 4e6ae40 drm/i915/bxt: Sanitize DC state tracking during system resume
> 5e7b5f6 drm/i915/bxt: Don't uninit/init display core twice during
> system suspend/resume
> b5a5dbd drm/i915: Uninline intel_suspend_complete
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx