From fbf6d8798fceb1f64eb0e5fd7cd541becfc376cd Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 23 Mar 2016 14:51:12 +0100 Subject: [PATCH] drm/i915: Add locking to pll updates, v3. 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 Link: http://patchwork.freedesktop.org/patch/msgid/56F29F50.1090708@linux.intel.com Reviewed-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ drivers/gpu/drm/i915/intel_dpll_mgr.c | 25 +++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f6d71590bd7b..86fafd88ad3b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -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]; diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 19bfe6743ef2..1175eebfe03b 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -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); -- GitLab