[v4,2/2] drm/i915: Shut off PW2 when changing cdclk on glk

Commit Message

Kumar, Abhay June 12, 2018, 9:58 p.m. UTC

From: Ville Syrjälä <[e-mail]>

Apparently the audio hardware gets confused if it's powered up when
change the cdclk frequency. Force PW2 (which is where audio lives)
off when we do the cdclk reprogramming.

This is a rather big hack. If something is using PW2 when we do this
things wil break. I don't think there should be anything active on
the display side since we've turned off all the pipes and we've locked
out gmbus and aux, but I may be overlooking something. The problem
is more on the audio side. If audio is active when we do this PW2
toggle I'm sure something "interesting" will happen. But presumably
something would also happen if we just changed cdclk without the PW2
toggle.

A better fix would involve somehow forcing everything to drop
their PW2 references, which isn't trivial. And to make the audio driver
participate in this scheme we'd definitely need some kind of pre/post
cdclk change notify hooks in the audio component so that i915 can
actually inform the audio driver that the cdclk is going to be changed.
Either that or the audio driver would have to promise never to touch
the hardware when the pipes are off (which is how the VLV/CHV LPE
audio driver works IIRC).

Even with this hacky scheme it would make more sense to me to have
the pre/post cdclk change hooks so that the audio driver is actually
informed when the cdclk change/pw2 toggle will occur. What the audio
driver would do to prepare itself I don't actually know.

Signed-off-by: Ville Syrjälä <[e-mail]>
Signed-off-by: Abhay Kumar <[e-mail]>
---
 drivers/gpu/drm/i915/intel_cdclk.c      | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h        |  5 +++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 34 +++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+)

Patch

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 0f0aea900ceb..6557f1e9cf9e 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1356,6 +1356,7 @@  static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
+	bool enable_pw2 = false;
 	u32 val, divider;
 	int ret;
 
@@ -1381,6 +1382,14 @@  static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 
 	/*
+	 * On GLK HDA apparently gets confused if
+	 * cdclk is changed while PW2 is on
+	 */
+	if (IS_GEMINILAKE(dev_priv))
+		enable_pw2 = intel_display_power_toggle_start(dev_priv,
+							      SKL_DISP_PW_2);
+
+	/*
 	 * Inform power controller of upcoming frequency change. BSpec
 	 * requires us to wait up to 150usec, but that leads to timeouts;
 	 * the 2ms used here is based on experiment.
@@ -1437,6 +1446,11 @@  static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 
 	intel_update_cdclk(dev_priv);
+
+	if (IS_GEMINILAKE(dev_priv))
+		intel_display_power_toggle_end(dev_priv,
+					       SKL_DISP_PW_2,
+					       enable_pw2);
 }
 
 static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0da17ad056ec..c4fc107ad8cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1950,6 +1950,11 @@  bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
 					enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
+bool intel_display_power_toggle_start(struct drm_i915_private *dev_priv,
+				      enum i915_power_well_id power_well_id);
+void intel_display_power_toggle_end(struct drm_i915_private *dev_priv,
+				    enum i915_power_well_id power_well_id,
+				    bool enable);
 void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 			    u8 req_slices);
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 53a6eaa9671a..86a4b788e224 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2809,6 +2809,40 @@  static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
 	usleep_range(10, 30);		/* 10 us delay per Bspec */
 }
 
+bool intel_display_power_toggle_start(struct drm_i915_private *dev_priv,
+				      enum i915_power_well_id power_well_id)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	struct i915_power_well *well = lookup_power_well(dev_priv, power_well_id);
+	bool was_enabled;
+
+	mutex_lock(&power_domains->lock);
+
+	was_enabled = well->hw_enabled;
+
+	if (was_enabled)
+		intel_power_well_disable(dev_priv, well);
+
+	return was_enabled;
+}
+
+void intel_display_power_toggle_end(struct drm_i915_private *dev_priv,
+				    enum i915_power_well_id power_well_id,
+				    bool enable)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	struct i915_power_well *well = lookup_power_well(dev_priv, power_well_id);
+
+	lockdep_assert_held(&power_domains->lock);
+
+	if (enable) {
+		WARN_ON(well->hw_enabled);
+		intel_power_well_enable(dev_priv, well);
+	}
+
+	mutex_unlock(&power_domains->lock);
+}
+
 void bxt_display_core_init(struct drm_i915_private *dev_priv,
 			   bool resume)
 {