[PATCH v4 0/4] Enable Dynamic cdclk and HDA together on GLK

[PATCH v4 0/4] Enable Dynamic cdclk and HDA together on GLK

[Abhay Kumar]

Patches needed to change cdclk to 2*BCLK before accessing HDA Codec.

Ville Syrjälä (4):
  drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
  drm/i915: Introduce for_each_intel_dp()
  drm/i915: Lock gmbus/aux mutexes while changing cdclk
  drm/i915: Shut off PW2 when changing cdclk on glk

 drivers/gpu/drm/i915/i915_drv.c         |  1 +
 drivers/gpu/drm/i915/i915_drv.h         |  3 ++
 drivers/gpu/drm/i915/i915_reg.h         |  4 ++
 drivers/gpu/drm/i915/intel_audio.c      | 67 ++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_cdclk.c      | 68 +++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_display.c    |  7 +++-
 drivers/gpu/drm/i915/intel_display.h    |  4 ++
 drivers/gpu/drm/i915/intel_dp.c         | 38 ++++--------------
 drivers/gpu/drm/i915/intel_drv.h        | 21 ++++++++++
 drivers/gpu/drm/i915/intel_i2c.c        |  1 -
 drivers/gpu/drm/i915/intel_runtime_pm.c | 34 +++++++++++++++++
 11 files changed, 191 insertions(+), 57 deletions(-)

-- 
2.7.4

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

[PATCH v4 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Abhay Kumar]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.

v2: Use atomic refcount for get_power, put_power so that we can
    call each once(Abhay).
v3: Reset power well 2 to avoid any transaction on iDisp link
    during cdclk change(Abhay).
v4: Remove Power well 2 reset workaround(Ville).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  3 ++
 drivers/gpu/drm/i915/i915_reg.h      |  4 +++
 drivers/gpu/drm/i915/intel_audio.c   | 67 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_cdclk.c   | 29 +++++-----------
 drivers/gpu/drm/i915/intel_display.c |  7 +++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 6 files changed, 87 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6104d7115054..a4a386a5db69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1702,6 +1702,7 @@ struct drm_i915_private {
 	unsigned int hpll_freq;
 	unsigned int fdi_pll_freq;
 	unsigned int czclk_freq;
+	u32 get_put_refcount;
 
 	struct {
 		/*
@@ -1719,6 +1720,8 @@ struct drm_i915_private {
 		struct intel_cdclk_state actual;
 		/* The current hardware cdclk state */
 		struct intel_cdclk_state hw;
+
+		int force_min_cdclk;
 	} cdclk;
 
 	/**
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 987def26ce82..cef770184245 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8869,6 +8869,10 @@ enum skl_power_gate {
  * SKL Clocks
  */
 
+/* Power well 2 */
+#define POWER_WELL_2			_MMIO(0x45404)
+#define POWER_WELL_2_REQUEST		(1<<31)
+
 /* CDCLK_CTL */
 #define CDCLK_CTL			_MMIO(0x46000)
 #define  CDCLK_FREQ_SEL_MASK		(3 << 26)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..ca8f04c7cbb3 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
 
 	if (!connector->eld[0])
 		return;
-
 	DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 			 connector->base.id,
 			 connector->name,
@@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+				bool enable)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_state *state;
+	int ret;
+
+	drm_modeset_acquire_init(&ctx, 0);
+	state = drm_atomic_state_alloc(&dev_priv->drm);
+	if (WARN_ON(!state))
+		return;
+
+	state->acquire_ctx = &ctx;
+
+retry:
+	to_intel_atomic_state(state)->modeset = true;
+	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+		enable ? 2 * 96000 : 0;
+
+	/*
+	 * Protects dev_priv->cdclk.force_min_cdclk
+	 * Need to lock this here in case we have no active pipes
+	 * and thus wouldn't lock it during the commit otherwise.
+	 */
+	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex, &ctx);
+	if (!ret)
+		ret = drm_atomic_commit(state);
+
+	if (ret == -EDEADLK) {
+		drm_atomic_state_clear(state);
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	WARN_ON(ret);
+
+	drm_atomic_state_put(state);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
 static void i915_audio_component_get_power(struct device *kdev)
 {
-	intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+	dev_priv->get_put_refcount++;
+
+	/* Force cdclk to 2*BCLK during first time get power call */
+	if (dev_priv->get_put_refcount == 1)
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			glk_force_audio_cdclk(dev_priv, true);
+
+	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
 }
 
 static void i915_audio_component_put_power(struct device *kdev)
 {
-	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+	dev_priv->get_put_refcount--;
+
+	/* Force required cdclk during last time put power call */
+	if (dev_priv->get_put_refcount == 0)
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			glk_force_audio_cdclk(dev_priv, false);
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
 }
 
 static void i915_audio_component_codec_wake_override(struct device *kdev,
@@ -959,7 +1018,7 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv)
 		/* continue with reduced functionality */
 		return;
 	}
-
+	dev_priv->get_put_refcount = 0;
 	dev_priv->audio_component_registered = true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 8ed7bd052e46..0f0aea900ceb 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2153,19 +2153,8 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	/*
 	 * According to BSpec, "The CD clock frequency must be at least twice
 	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
-	 *
-	 * FIXME: Check the actual, not default, BCLK being used.
-	 *
-	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
-	 * is required for audio probe, also when there are no audio capable
-	 * displays connected at probe time. This leads to unnecessarily high
-	 * CDCLK when audio is not required.
-	 *
-	 * FIXME: This limit is only applied when there are displays connected
-	 * at probe time. If we probe without displays, we'll still end up using
-	 * the platform minimum CDCLK, failing audio probe.
 	 */
-	if (INTEL_GEN(dev_priv) >= 9)
+	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
 		min_cdclk = max(2 * 96000, min_cdclk);
 
 	/*
@@ -2205,7 +2194,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
 		intel_state->min_cdclk[i] = min_cdclk;
 	}
 
-	min_cdclk = 0;
+	min_cdclk = intel_state->cdclk.force_min_cdclk;
 	for_each_pipe(dev_priv, pipe)
 		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
 
@@ -2266,7 +2255,7 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 		vlv_calc_voltage_level(dev_priv, cdclk);
 
 	if (!intel_state->active_crtcs) {
-		cdclk = vlv_calc_cdclk(dev_priv, 0);
+		cdclk = vlv_calc_cdclk(dev_priv, intel_state->cdclk.force_min_cdclk);
 
 		intel_state->cdclk.actual.cdclk = cdclk;
 		intel_state->cdclk.actual.voltage_level =
@@ -2299,7 +2288,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 		bdw_calc_voltage_level(cdclk);
 
 	if (!intel_state->active_crtcs) {
-		cdclk = bdw_calc_cdclk(0);
+		cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 
 		intel_state->cdclk.actual.cdclk = cdclk;
 		intel_state->cdclk.actual.voltage_level =
@@ -2371,7 +2360,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 		skl_calc_voltage_level(cdclk);
 
 	if (!intel_state->active_crtcs) {
-		cdclk = skl_calc_cdclk(0, vco);
+		cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
 
 		intel_state->cdclk.actual.vco = vco;
 		intel_state->cdclk.actual.cdclk = cdclk;
@@ -2410,10 +2399,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 	if (!intel_state->active_crtcs) {
 		if (IS_GEMINILAKE(dev_priv)) {
-			cdclk = glk_calc_cdclk(0);
+			 cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 			vco = glk_de_pll_vco(dev_priv, cdclk);
 		} else {
-			cdclk = bxt_calc_cdclk(0);
+			cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 			vco = bxt_de_pll_vco(dev_priv, cdclk);
 		}
 
@@ -2449,7 +2438,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
 		    cnl_compute_min_voltage_level(intel_state));
 
 	if (!intel_state->active_crtcs) {
-		cdclk = cnl_calc_cdclk(0);
+		cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 		vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
 
 		intel_state->cdclk.actual.vco = vco;
@@ -2482,7 +2471,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	intel_state->cdclk.logical.cdclk = cdclk;
 
 	if (!intel_state->active_crtcs) {
-		cdclk = icl_calc_cdclk(0, ref);
+		cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
 		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
 
 		intel_state->cdclk.actual.vco = vco;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 17c590b42fd7..3ee1c1f5419d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12162,6 +12162,10 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 		return -EINVAL;
 	}
 
+	/* keep the current setting */
+	if (!intel_state->modeset)
+		intel_state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk;
+
 	intel_state->modeset = true;
 	intel_state->active_crtcs = dev_priv->active_crtcs;
 	intel_state->cdclk.logical = dev_priv->cdclk.logical;
@@ -12257,7 +12261,7 @@ static int intel_atomic_check(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *crtc_state;
 	int ret, i;
-	bool any_ms = false;
+	bool any_ms = intel_state->modeset;
 
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
@@ -12805,6 +12809,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		dev_priv->active_crtcs = intel_state->active_crtcs;
 		dev_priv->cdclk.logical = intel_state->cdclk.logical;
 		dev_priv->cdclk.actual = intel_state->cdclk.actual;
+		dev_priv->cdclk.force_min_cdclk = intel_state->cdclk.force_min_cdclk;
 	}
 
 	drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8641583842be..0da17ad056ec 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -459,6 +459,8 @@ struct intel_atomic_state {
 		 * state only when all crtc's are DPMS off.
 		 */
 		struct intel_cdclk_state actual;
+
+		int force_min_cdclk;
 	} cdclk;
 
 	bool dpll_set, modeset;
-- 
2.7.4

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

[PATCH v4 2/4] drm/i915: Introduce for_each_intel_dp()

[Abhay Kumar]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a convenience macro for iterating DP encoders.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.h |  4 ++++
 drivers/gpu/drm/i915/intel_dp.c      | 38 +++++++-----------------------------
 drivers/gpu/drm/i915/intel_drv.h     | 14 +++++++++++++
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index c88185ed7594..e153d0e925d8 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -284,6 +284,10 @@ struct intel_link_m_n {
 			    &(dev)->mode_config.encoder_list,	\
 			    base.head)
 
+#define for_each_intel_dp(dev, intel_encoder)			\
+	for_each_intel_encoder(dev, intel_encoder)		\
+		for_each_if(intel_encoder_is_dp(intel_encoder))
+
 #define for_each_intel_connector_iter(intel_connector, iter) \
 	while ((intel_connector = to_intel_connector(drm_connector_list_iter_next(iter))))
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 37b9f62aeb6e..fcc7c5465d48 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -583,14 +583,8 @@ static enum pipe vlv_find_free_pps(struct drm_i915_private *dev_priv)
 	 * We don't have power sequencer currently.
 	 * Pick one that's not used by other ports.
 	 */
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		struct intel_dp *intel_dp;
-
-		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP)
-			continue;
-
-		intel_dp = enc_to_intel_dp(&encoder->base);
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
 		if (encoder->type == INTEL_OUTPUT_EDP) {
 			WARN_ON(intel_dp->active_pipe != INVALID_PIPE &&
@@ -782,19 +776,8 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
 	 * should use them always.
 	 */
 
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		struct intel_dp *intel_dp;
-
-		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP &&
-		    encoder->type != INTEL_OUTPUT_DDI)
-			continue;
-
-		intel_dp = enc_to_intel_dp(&encoder->base);
-
-		/* Skip pure DVI/HDMI DDI encoders */
-		if (!i915_mmio_reg_valid(intel_dp->output_reg))
-			continue;
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
 		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
 
@@ -3078,16 +3061,9 @@ static void vlv_steal_power_sequencer(struct drm_i915_private *dev_priv,
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		struct intel_dp *intel_dp;
-		enum port port;
-
-		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP)
-			continue;
-
-		intel_dp = enc_to_intel_dp(&encoder->base);
-		port = dp_to_dig_port(intel_dp)->base.port;
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+		enum port port = encoder->port;
 
 		WARN(intel_dp->active_pipe == pipe,
 		     "stealing pipe %c power sequencer from active (e)DP port %c\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0da17ad056ec..c77942adda22 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1282,6 +1282,20 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 	return &enc_to_dig_port(encoder)->dp;
 }
 
+static inline bool intel_encoder_is_dp(struct intel_encoder *encoder)
+{
+	switch (encoder->type) {
+	case INTEL_OUTPUT_DP:
+	case INTEL_OUTPUT_EDP:
+		return true;
+	case INTEL_OUTPUT_DDI:
+		/* Skip pure HDMI/DVI DDI encoders */
+		return i915_mmio_reg_valid(enc_to_intel_dp(&encoder->base)->output_reg);
+	default:
+		return false;
+	}
+}
+
 static inline struct intel_digital_port *
 dp_to_dig_port(struct intel_dp *intel_dp)
 {
-- 
2.7.4

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

[PATCH v4 3/4] drm/i915: Lock gmbus/aux mutexes while changing cdclk

[Abhay Kumar]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

gmbus/aux may be clocked by cdclk, thus we should make sure no
transfers are ongoing while the cdclk frequency is being changed.
We do that by simply grabbing all the gmbus/aux mutexes. No one
else should be holding any more than one of those at a time so
the lock ordering here shouldn't matter.

An alternative apporach would be the introduction of a cdclk
rwsem. Cdclk reprogramming would take the write lock, all users
of cdclk would take the read lock.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c    |  1 +
 drivers/gpu/drm/i915/intel_cdclk.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_i2c.c   |  1 -
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4cdd70de5ed0..2a30369b9df9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -899,6 +899,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	mutex_init(&dev_priv->av_mutex);
 	mutex_init(&dev_priv->wm.wm_mutex);
 	mutex_init(&dev_priv->pps_mutex);
+	mutex_init(&dev_priv->gmbus_mutex);
 
 	i915_memcpy_init_early(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 0f0aea900ceb..ebfafef7bf88 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2078,6 +2078,9 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 void intel_set_cdclk(struct drm_i915_private *dev_priv,
 		     const struct intel_cdclk_state *cdclk_state)
 {
+	struct intel_encoder *encoder;
+	unsigned int aux_mutex_lockclass = 0;
+
 	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
 		return;
 
@@ -2086,8 +2089,30 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 
 	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
 
+	/*
+	 * Lock aux/gmbus while we change cdclk in case the
+	 * those functions use cdclk. Not all platforms/ports
+	 * do, but we'll lock them all for simplicity. All other
+	 * users of cdclk (apart from audio) should be off on
+	 * account of the pipes being off.
+	 */
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+		mutex_lock_nested(&intel_dp->aux.hw_mutex,
+				  aux_mutex_lockclass++);
+	}
+	mutex_lock(&dev_priv->gmbus_mutex);
+
 	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
 
+	for_each_intel_dp(&dev_priv->drm, encoder) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+		mutex_unlock(&intel_dp->aux.hw_mutex);
+	}
+	mutex_unlock(&dev_priv->gmbus_mutex);
+
 	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
 		 "cdclk state doesn't match!\n")) {
 		intel_dump_cdclk_state(&dev_priv->cdclk.hw, "[hw state]");
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 61729bf84e08..14bc8889596e 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -781,7 +781,6 @@ int intel_setup_gmbus(struct drm_i915_private *dev_priv)
 			i915_mmio_reg_offset(PCH_GPIOA) -
 			i915_mmio_reg_offset(GPIOA);
 
-	mutex_init(&dev_priv->gmbus_mutex);
 	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
 
 	for (pin = 0; pin < ARRAY_SIZE(dev_priv->gmbus); pin++) {
-- 
2.7.4

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

[PATCH v4 4/4] drm/i915: Shut off PW2 when changing cdclk on glk

[Abhay Kumar]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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ä <ville.syrjala@linux.intel.com>
---
 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(+)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index ebfafef7bf88..206f573c89b1 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 c77942adda22..e92ea5eff46f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1964,6 +1964,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)
 {
-- 
2.7.4

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

✗ Fi.CI.CHECKPATCH: warning for Enable Dynamic cdclk and HDA together on GLK

[Patchwork]

== Series Details ==

Series: Enable Dynamic cdclk and HDA together on GLK
URL   : https://patchwork.freedesktop.org/series/44713/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
08c10018dfc7 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
-:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#54: FILE: drivers/gpu/drm/i915/i915_reg.h:8886:
+#define POWER_WELL_2_REQUEST		(1<<31)
                             		  ^

-:76: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#76: FILE: drivers/gpu/drm/i915/intel_audio.c:716:
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+				bool enable)

-:221: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 25)
#221: FILE: drivers/gpu/drm/i915/intel_cdclk.c:2401:
 		if (IS_GEMINILAKE(dev_priv)) {
+			 cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);

total: 0 errors, 1 warnings, 2 checks, 229 lines checked
b521f169b581 drm/i915: Introduce for_each_intel_dp()
-:21: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#21: FILE: drivers/gpu/drm/i915/intel_display.h:288:
+#define for_each_intel_dp(dev, intel_encoder)			\
+	for_each_intel_encoder(dev, intel_encoder)		\
+		for_each_if(intel_encoder_is_dp(intel_encoder))

-:21: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'intel_encoder' - possible side-effects?
#21: FILE: drivers/gpu/drm/i915/intel_display.h:288:
+#define for_each_intel_dp(dev, intel_encoder)			\
+	for_each_intel_encoder(dev, intel_encoder)		\
+		for_each_if(intel_encoder_is_dp(intel_encoder))

total: 1 errors, 0 warnings, 1 checks, 86 lines checked
5e2850c0c5ad drm/i915: Lock gmbus/aux mutexes while changing cdclk
2fde8d7211a8 drm/i915: Shut off PW2 when changing cdclk on glk

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

✗ Fi.CI.SPARSE: warning for Enable Dynamic cdclk and HDA together on GLK

[Patchwork]

== Series Details ==

Series: Enable Dynamic cdclk and HDA together on GLK
URL   : https://patchwork.freedesktop.org/series/44713/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
-O:drivers/gpu/drm/i915/intel_cdclk.c:2169:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2158:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_cdclk.c:2210:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_cdclk.c:2210:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2199:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2199:29: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3681:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3684:16: warning: expression using sizeof(void)

Commit: drm/i915: Introduce for_each_intel_dp()
Okay!

Commit: drm/i915: Lock gmbus/aux mutexes while changing cdclk
Okay!

Commit: drm/i915: Shut off PW2 when changing cdclk on glk
Okay!

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

✓ Fi.CI.BAT: success for Enable Dynamic cdclk and HDA together on GLK

[Patchwork]

== Series Details ==

Series: Enable Dynamic cdclk and HDA together on GLK
URL   : https://patchwork.freedesktop.org/series/44713/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4311 -> Patchwork_9293 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44713/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9293 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097) +1

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       PASS -> DMESG-FAIL (fdo#106103, fdo#102614)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cnl-psr:         FAIL (fdo#100368) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (43 -> 39) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


== Build changes ==

    * Linux: CI_DRM_4311 -> Patchwork_9293

  CI_DRM_4311: 875eebee4a444f5b7ba146754e91118ff3c11ad5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4518: e4908004547b63131352fbc0ddcdb1d3d55480e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9293: 2fde8d7211a81ad3f1360aaeb17484b1b43bdef8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2fde8d7211a8 drm/i915: Shut off PW2 when changing cdclk on glk
5e2850c0c5ad drm/i915: Lock gmbus/aux mutexes while changing cdclk
b521f169b581 drm/i915: Introduce for_each_intel_dp()
08c10018dfc7 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9293/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for Enable Dynamic cdclk and HDA together on GLK

[Patchwork]

== Series Details ==

Series: Enable Dynamic cdclk and HDA together on GLK
URL   : https://patchwork.freedesktop.org/series/44713/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4311_full -> Patchwork_9293_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9293_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9293_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9293_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_rotation_crc@sprite-rotation-90:
      shard-glk:          PASS -> DMESG-WARN +4

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          PASS -> SKIP +2

    igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
      shard-kbl:          SKIP -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9293_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-glk:          PASS -> FAIL (fdo#105703)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#102887, fdo#105363)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103822)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724)

    igt@kms_plane_lowres@pipe-c-tiling-none:
      shard-glk:          PASS -> DMESG-FAIL (fdo#104724, fdo#103166) +2

    igt@kms_vblank@pipe-b-wait-forked-busy-hang:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763) +5

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          FAIL (fdo#105347) -> PASS

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS

    igt@kms_setmode@basic:
      shard-glk:          FAIL (fdo#99912) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-glk:          FAIL (fdo#105347) -> INCOMPLETE (fdo#103359, k.org#198133)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4311 -> Patchwork_9293

  CI_DRM_4311: 875eebee4a444f5b7ba146754e91118ff3c11ad5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4518: e4908004547b63131352fbc0ddcdb1d3d55480e0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9293: 2fde8d7211a81ad3f1360aaeb17484b1b43bdef8 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9293/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 0/4] Enable Dynamic cdclk and HDA together on GLK

[Kumar, Abhay]

Hi Ville,

Looks like we have right fix from audio and we will not need powerwell 2 
reset workaround when changing cdclk.

we need only one patch in this series to get merged.

drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled.

Regards,
Abhay



On 6/13/2018 11:41 AM, Abhay Kumar wrote:
> Patches needed to change cdclk to 2*BCLK before accessing HDA Codec.
>
> Ville Syrjälä (4):
>    drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
>    drm/i915: Introduce for_each_intel_dp()
>    drm/i915: Lock gmbus/aux mutexes while changing cdclk
>    drm/i915: Shut off PW2 when changing cdclk on glk
>
>   drivers/gpu/drm/i915/i915_drv.c         |  1 +
>   drivers/gpu/drm/i915/i915_drv.h         |  3 ++
>   drivers/gpu/drm/i915/i915_reg.h         |  4 ++
>   drivers/gpu/drm/i915/intel_audio.c      | 67 ++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/i915/intel_cdclk.c      | 68 +++++++++++++++++++++++----------
>   drivers/gpu/drm/i915/intel_display.c    |  7 +++-
>   drivers/gpu/drm/i915/intel_display.h    |  4 ++
>   drivers/gpu/drm/i915/intel_dp.c         | 38 ++++--------------
>   drivers/gpu/drm/i915/intel_drv.h        | 21 ++++++++++
>   drivers/gpu/drm/i915/intel_i2c.c        |  1 -
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 34 +++++++++++++++++
>   11 files changed, 191 insertions(+), 57 deletions(-)
>

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