drm/i915: Add locking to pll updates, v3.
authorMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Wed, 23 Mar 2016 13:51:12 +0000 (14:51 +0100)
committerMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Thu, 31 Mar 2016 07:43:11 +0000 (09:43 +0200)
With async modesets this is no longer protected with connection_mutex,
so ensure that each pll has its own lock. The pll configuration state
is still protected; it's only the pll updates that need locking against
concurrency.

Changes since v1:
- Rebased.
- Fix locking to protect all accesses. (Durgadoss)
Changes since v2:
- Make the dpll_lock global to protect concurrent updates to the
  same register, for example DPLL_CTRL1 on skl. (Ander)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/56F29F50.1090708@linux.intel.com
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/intel_dpll_mgr.c

index f6d71590bd7b6648c0e1345371d4ba2fd35e12f6..86fafd88ad3b8156677c581df57f82b6ddba9b51 100644 (file)
@@ -1837,6 +1837,13 @@ struct drm_i915_private {
        struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
        const struct intel_dpll_mgr *dpll_mgr;
 
+       /*
+        * dpll_lock serializes intel_{prepare,enable,disable}_shared_dpll.
+        * Must be global rather than per dpll, because on some platforms
+        * plls share registers.
+        */
+       struct mutex dpll_lock;
+
        unsigned int active_crtcs;
        unsigned int min_pixclk[I915_MAX_PIPES];
 
index 19bfe6743ef23f8577a44670e1bfe86218942689..1175eebfe03b6463ce87c4b4e7be1b752d0b4970 100644 (file)
@@ -89,14 +89,16 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc)
        if (WARN_ON(pll == NULL))
                return;
 
+       mutex_lock(&dev_priv->dpll_lock);
        WARN_ON(!pll->config.crtc_mask);
-       if (pll->active_mask == 0) {
+       if (!pll->active_mask) {
                DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
                WARN_ON(pll->on);
                assert_shared_dpll_disabled(dev_priv, pll);
 
                pll->funcs.mode_set(dev_priv, pll);
        }
+       mutex_unlock(&dev_priv->dpll_lock);
 }
 
 /**
@@ -113,14 +115,17 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
        struct drm_i915_private *dev_priv = dev->dev_private;
        struct intel_shared_dpll *pll = crtc->config->shared_dpll;
        unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base);
-       unsigned old_mask = pll->active_mask;
+       unsigned old_mask;
 
        if (WARN_ON(pll == NULL))
                return;
 
+       mutex_lock(&dev_priv->dpll_lock);
+       old_mask = pll->active_mask;
+
        if (WARN_ON(!(pll->config.crtc_mask & crtc_mask)) ||
            WARN_ON(pll->active_mask & crtc_mask))
-               return;
+               goto out;
 
        pll->active_mask |= crtc_mask;
 
@@ -131,13 +136,16 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
        if (old_mask) {
                WARN_ON(!pll->on);
                assert_shared_dpll_enabled(dev_priv, pll);
-               return;
+               goto out;
        }
        WARN_ON(pll->on);
 
        DRM_DEBUG_KMS("enabling %s\n", pll->name);
        pll->funcs.enable(dev_priv, pll);
        pll->on = true;
+
+out:
+       mutex_unlock(&dev_priv->dpll_lock);
 }
 
 void intel_disable_shared_dpll(struct intel_crtc *crtc)
@@ -154,8 +162,9 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc)
        if (pll == NULL)
                return;
 
+       mutex_lock(&dev_priv->dpll_lock);
        if (WARN_ON(!(pll->active_mask & crtc_mask)))
-               return;
+               goto out;
 
        DRM_DEBUG_KMS("disable %s (active %x, on? %d) for crtc %d\n",
                      pll->name, pll->active_mask, pll->on,
@@ -166,11 +175,14 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc)
 
        pll->active_mask &= ~crtc_mask;
        if (pll->active_mask)
-               return;
+               goto out;
 
        DRM_DEBUG_KMS("disabling %s\n", pll->name);
        pll->funcs.disable(dev_priv, pll);
        pll->on = false;
+
+out:
+       mutex_unlock(&dev_priv->dpll_lock);
 }
 
 static struct intel_shared_dpll *
@@ -1750,6 +1762,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
 
        dev_priv->dpll_mgr = dpll_mgr;
        dev_priv->num_shared_dpll = i;
+       mutex_init(&dev_priv->dpll_lock);
 
        BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);