[PATCH] drm/i915/skl: Move DC6 entry from PW1 to PW0/device level

[Daniel Vetter <daniel.vetter@ffwll.ch>]

From: Animesh Manna <animesh.manna@intel.com>

Mmio register access after DC6 entry is not allowed when according to
bspec (bspec-id 0527). Hence we must move dc6 enabling to be the very
last/first thing in the device suspend/resume flow. Currently it is
bound to PW2 as an alternative to DC5.

With this patch we have the following power wells for runtime pm on
skl:
- device-level/PW0, newly coupled with dc6
- PW1, which is a no-op power domain since DMC manages PW1 on it's
  own.
- PW2, which is coupled with DC5 now. Note that DC5 can only be
  entered when PW1 is off, but the DMC takes care of that for us.

Ville and Imre have a long-term plan which will split out DC6 and DC5
into their own levels and remove the defunct PW1. See

http://lists.freedesktop.org/archives/intel-gfx/2015-September/076612.html

Without this patch DMC freaks out and we fail to reach PC10.

v1: Initial version.

v2: Based on review comment from Daniel,
- created a seperate patch for csr uninitialization set call.

v3: Rebased on top of latest code.

v4: Completely new commit message.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.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> (v3)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

---

Anyway this was my best attempt at trying to explain why the code
changes are correct, but when I wanted Ville to cross-check this on
irc he told me that it's bonkers. Apparently we need to enable dc6
somewhere in between pw0 and dc5 for real, and then when we go into
entire device suspend mode we need to disable it again for c9-10
according to specs.

The other problem is that this has only been discussed between
Imre/Ville/Patrik to resolve some of the other dc5/6 issues we still
have open (dp aux/gmbus interactions). And there was no input at all
afaics from vpg display, so we have a full-blown disconnect here.

I have now fucking idea at all any more what I'm supposed to do. But
please start talking with each another.

I'm just sending this out as for the record but wont apply it.

Thanks, Daniel
---
 drivers/gpu/drm/i915/i915_drv.c         | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_drv.h        |  2 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++------------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f9ed72726661..09043c50f5d8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1049,10 +1049,20 @@ static int i915_pm_resume(struct device *dev)
 
 static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
+	enum csr_state state;
 	/* Enabling DC6 is not a hard requirement to enter runtime D3 */
 
 	skl_uninit_cdclk(dev_priv);
 
+	/* TODO: wait for a completion event or
+	 * similar here instead of busy
+	 * waiting using wait_for function.
+	 */
+	wait_for((state = intel_csr_load_status_get(dev_priv)) !=
+			FW_UNINITIALIZED, 1000);
+	if (state == FW_LOADED)
+		skl_enable_dc6(dev_priv);
+
 	return 0;
 }
 
@@ -1099,6 +1109,9 @@ static 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) == FW_LOADED)
+		skl_disable_dc6(dev_priv);
+
 	skl_init_cdclk(dev_priv);
 	intel_csr_load_program(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d5cac8d53c18..0bc2e3d325ee 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1145,6 +1145,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 e1fdbabaf2bf..f03a0e266240 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -550,7 +550,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;
 
@@ -567,7 +567,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;
 
@@ -628,10 +628,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.
@@ -639,8 +639,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);
@@ -666,7 +664,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
 			}
 
-			if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) &&
+			if (GEN9_ENABLE_DC5(dev) &&
 				power_well->data == SKL_DISP_PW_2) {
 				enum csr_state state;
 				/* TODO: wait for a completion event or
@@ -679,10 +677,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 					DRM_DEBUG("CSR firmware not ready (%d)\n",
 							state);
 				else
-					if (SKL_ENABLE_DC6(dev))
-						skl_enable_dc6(dev_priv);
-					else
-						gen9_enable_dc5(dev_priv);
+					gen9_enable_dc5(dev_priv);
 			}
 		}
 	}
-- 
2.5.1

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