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

Commit Message

Kumar, Abhay June 13, 2018, 6:41 p.m. UTC

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

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ä <[e-mail]>
---
 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(-)

Patch

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++) {