>-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Maarten Lankhorst >Sent: Monday, February 29, 2016 6:22 PM >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Subject: [PATCH 1/4] drm/i915: Use a crtc mask instead of a refcount for dpll functions. > >This makes it easier to verify correct dpll setup with only a single crtc. >It it also useful to detect double dpll enable/disable. s/it/is Actually, a welcome change ;-) Reviewed-by: Durgadoss R <durgadoss.r@xxxxxxxxx> Thanks, Durga > >Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >--- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 68 ++++++++++++++++++++---------------- > 3 files changed, 40 insertions(+), 32 deletions(-) > >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >index c6e64e2c951e..200e45922587 100644 >--- a/drivers/gpu/drm/i915/i915_debugfs.c >+++ b/drivers/gpu/drm/i915/i915_debugfs.c >@@ -3211,7 +3211,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) > > seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id); > seq_printf(m, " crtc_mask: 0x%08x, active: %d, on: %s\n", >- pll->config.crtc_mask, pll->active, yesno(pll->on)); >+ pll->config.crtc_mask, hweight32(pll->active_mask), yesno(pll->on)); > seq_printf(m, " tracked hardware state:\n"); > seq_printf(m, " dpll: 0x%08x\n", pll->config.hw_state.dpll); > seq_printf(m, " dpll_md: 0x%08x\n", >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >index 10480939159c..c7401b50818c 100644 >--- a/drivers/gpu/drm/i915/i915_drv.h >+++ b/drivers/gpu/drm/i915/i915_drv.h >@@ -392,7 +392,7 @@ struct intel_shared_dpll_config { > struct intel_shared_dpll { > struct intel_shared_dpll_config config; > >- int active; /* count of number of active CRTCs (i.e. DPMS on) */ >+ unsigned active_mask; /* mask of active CRTCs (i.e. DPMS on) */ > bool on; /* is the PLL actually active? Disabled during modeset */ > const char *name; > /* should match the index in the dev_priv->shared_dplls array */ >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >index a5d61084ae98..5dd59cae9f06 100644 >--- a/drivers/gpu/drm/i915/intel_display.c >+++ b/drivers/gpu/drm/i915/intel_display.c >@@ -1866,7 +1866,7 @@ static void intel_prepare_shared_dpll(struct intel_crtc *crtc) > return; > > WARN_ON(!pll->config.crtc_mask); >- if (pll->active == 0) { >+ if (pll->active_mask == 0) { > DRM_DEBUG_DRIVER("setting up %s\n", pll->name); > WARN_ON(pll->on); > assert_shared_dpll_disabled(dev_priv, pll); >@@ -1888,6 +1888,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc) > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); >+ unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base); > > if (WARN_ON(pll == NULL)) > return; >@@ -1895,11 +1896,16 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc) > if (WARN_ON(pll->config.crtc_mask == 0)) > return; > >+ if (WARN_ON(pll->active_mask & crtc_mask)) >+ return; >+ > DRM_DEBUG_KMS("enable %s (active %d, on? %d) for crtc %d\n", >- pll->name, pll->active, pll->on, >+ pll->name, hweight32(pll->active_mask), pll->on, > crtc->base.base.id); > >- if (pll->active++) { >+ pll->active_mask |= crtc_mask; >+ >+ if (pll->active_mask != crtc_mask) { > WARN_ON(!pll->on); > assert_shared_dpll_enabled(dev_priv, pll); > return; >@@ -1918,30 +1924,33 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc) > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); >+ unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base); > > /* PCH only available on ILK+ */ >- if (INTEL_INFO(dev)->gen < 5) >+ if (INTEL_INFO(dev_priv)->gen < 5) > return; > > if (pll == NULL) > return; > >- if (WARN_ON(!(pll->config.crtc_mask & (1 << drm_crtc_index(&crtc->base))))) >+ if (WARN_ON(!(pll->config.crtc_mask & crtc_mask))) > return; > >- DRM_DEBUG_KMS("disable %s (active %d, on? %d) for crtc %d\n", >- pll->name, pll->active, pll->on, >+ if (WARN_ON(!(pll->active_mask & crtc_mask))) >+ return; >+ >+ DRM_DEBUG_KMS("disable %s (active %x, on? %d) for crtc %d\n", >+ pll->name, pll->active_mask, pll->on, > crtc->base.base.id); > >- if (WARN_ON(pll->active == 0)) { >+ pll->active_mask &= ~crtc_mask; >+ if (pll->active_mask) { > assert_shared_dpll_disabled(dev_priv, pll); > return; > } > > assert_shared_dpll_enabled(dev_priv, pll); > WARN_ON(!pll->on); >- if (--pll->active) >- return; > > DRM_DEBUG_KMS("disabling %s\n", pll->name); > pll->disable(dev_priv, pll); >@@ -4294,10 +4303,10 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, > if (memcmp(&crtc_state->dpll_hw_state, > &shared_dpll[i].hw_state, > sizeof(crtc_state->dpll_hw_state)) == 0) { >- DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative >%d)\n", >+ DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, active >0x%08x)\n", > crtc->base.base.id, pll->name, > shared_dpll[i].crtc_mask, >- pll->active); >+ pll->active_mask); > goto found; > } > } >@@ -12952,12 +12961,12 @@ check_shared_dpll_state(struct drm_device *dev) > > active = pll->get_hw_state(dev_priv, pll, &dpll_hw_state); > >- I915_STATE_WARN(pll->active > hweight32(pll->config.crtc_mask), >- "more active pll users than references: %i vs %i\n", >- pll->active, hweight32(pll->config.crtc_mask)); >- I915_STATE_WARN(pll->active && !pll->on, >+ I915_STATE_WARN(pll->active_mask & ~pll->config.crtc_mask, >+ "more active pll users than references: %x vs %x\n", >+ pll->active_mask, pll->config.crtc_mask); >+ I915_STATE_WARN(pll->active_mask && !pll->on, > "pll in active use but not on in sw tracking\n"); >- I915_STATE_WARN(pll->on && !pll->active, >+ I915_STATE_WARN(pll->on && !pll->active_mask, > "pll in on but not on in use in sw tracking\n"); > I915_STATE_WARN(pll->on != active, > "pll on state mismatch (expected %i, found %i)\n", >@@ -12965,16 +12974,16 @@ check_shared_dpll_state(struct drm_device *dev) > > for_each_intel_crtc(dev, crtc) { > if (crtc->base.state->enable && intel_crtc_to_shared_dpll(crtc) == pll) >- enabled_crtcs++; >+ enabled_crtcs |= 1 << drm_crtc_index(&crtc->base); > if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) >- active_crtcs++; >+ active_crtcs |= 1 << drm_crtc_index(&crtc->base); > } >- I915_STATE_WARN(pll->active != active_crtcs, >- "pll active crtcs mismatch (expected %i, found %i)\n", >- pll->active, active_crtcs); >- I915_STATE_WARN(hweight32(pll->config.crtc_mask) != enabled_crtcs, >- "pll enabled crtcs mismatch (expected %i, found %i)\n", >- hweight32(pll->config.crtc_mask), enabled_crtcs); >+ I915_STATE_WARN(pll->active_mask != active_crtcs, >+ "pll active crtcs mismatch (expected %x, found %x)\n", >+ pll->active_mask, active_crtcs); >+ I915_STATE_WARN(pll->config.crtc_mask != enabled_crtcs, >+ "pll enabled crtcs mismatch (expected %x, found %x)\n", >+ pll->config.crtc_mask, enabled_crtcs); > > I915_STATE_WARN(pll->on && memcmp(&pll->config.hw_state, &dpll_hw_state, > sizeof(dpll_hw_state)), >@@ -15760,14 +15769,13 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > > pll->on = pll->get_hw_state(dev_priv, pll, > &pll->config.hw_state); >- pll->active = 0; >+ pll->active_mask = 0; > pll->config.crtc_mask = 0; > for_each_intel_crtc(dev, crtc) { >- if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) { >- pll->active++; >+ if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) > pll->config.crtc_mask |= 1 << crtc->pipe; >- } > } >+ pll->active_mask = pll->config.crtc_mask; > > DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n", > pll->name, pll->config.crtc_mask, pll->on); >@@ -15889,7 +15897,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev) > for (i = 0; i < dev_priv->num_shared_dpll; i++) { > struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i]; > >- if (!pll->on || pll->active) >+ if (!pll->on || pll->active_mask) > continue; > > DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name); >-- >2.1.0 > >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx