drm/i915: Use pipes instead crtc indices in PLL state tracking

All the other places we have use pipes instead of crtc indices
when tracking resource usage. Life is easier when we do it
the same way always, so switch the dpll mgr to using pipes as
well. Looks like it was actually mixing these up in some cases
so it would not even have worked correctly except when the
device has a contiguous set of pipes starting from pipe A.
Granted, that is the typical case but supposedly it may not
always hold on modern hw.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210224144214.24803-4-ville.syrjala@linux.intel.com
Reviewed-by: Mika Kahola <mika.kahola@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ef5ce92819848da9bfbf5dceb5bd306fe3b5cc8f..78817d0c82e2cb891c8b39653a87e4406174ada2 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9643,7 +9643,7 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
                         struct intel_crtc_state *new_crtc_state)
 {
        struct intel_dpll_hw_state dpll_hw_state;
-       unsigned int crtc_mask;
+       u8 pipe_mask;
        bool active;
 
        memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
@@ -9656,34 +9656,34 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
                I915_STATE_WARN(!pll->on && pll->active_mask,
                     "pll in active use but not on in sw tracking\n");
                I915_STATE_WARN(pll->on && !pll->active_mask,
-                    "pll is on but not used by any active crtc\n");
+                    "pll is on but not used by any active pipe\n");
                I915_STATE_WARN(pll->on != active,
                     "pll on state mismatch (expected %i, found %i)\n",
                     pll->on, active);
        }
 
        if (!crtc) {
-               I915_STATE_WARN(pll->active_mask & ~pll->state.crtc_mask,
-                               "more active pll users than references: %x vs %x\n",
-                               pll->active_mask, pll->state.crtc_mask);
+               I915_STATE_WARN(pll->active_mask & ~pll->state.pipe_mask,
+                               "more active pll users than references: 0x%x vs 0x%x\n",
+                               pll->active_mask, pll->state.pipe_mask);
 
                return;
        }
 
-       crtc_mask = drm_crtc_mask(&crtc->base);
+       pipe_mask = BIT(crtc->pipe);
 
        if (new_crtc_state->hw.active)
-               I915_STATE_WARN(!(pll->active_mask & crtc_mask),
-                               "pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
+               I915_STATE_WARN(!(pll->active_mask & pipe_mask),
+                               "pll active mismatch (expected pipe %c in active mask 0x%x)\n",
                                pipe_name(crtc->pipe), pll->active_mask);
        else
-               I915_STATE_WARN(pll->active_mask & crtc_mask,
-                               "pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
+               I915_STATE_WARN(pll->active_mask & pipe_mask,
+                               "pll active mismatch (didn't expect pipe %c in active mask 0x%x)\n",
                                pipe_name(crtc->pipe), pll->active_mask);
 
-       I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
-                       "pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
-                       crtc_mask, pll->state.crtc_mask);
+       I915_STATE_WARN(!(pll->state.pipe_mask & pipe_mask),
+                       "pll enabled crtcs mismatch (expected 0x%x in 0x%x)\n",
+                       pipe_mask, pll->state.pipe_mask);
 
        I915_STATE_WARN(pll->on && memcmp(&pll->state.hw_state,
                                          &dpll_hw_state,
@@ -9703,15 +9703,15 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
 
        if (old_crtc_state->shared_dpll &&
            old_crtc_state->shared_dpll != new_crtc_state->shared_dpll) {
-               unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
+               u8 pipe_mask = BIT(crtc->pipe);
                struct intel_shared_dpll *pll = old_crtc_state->shared_dpll;
 
-               I915_STATE_WARN(pll->active_mask & crtc_mask,
-                               "pll active mismatch (didn't expect pipe %c in active mask)\n",
-                               pipe_name(crtc->pipe));
-               I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
-                               "pll enabled crtcs mismatch (found %x in enabled mask)\n",
-                               pipe_name(crtc->pipe));
+               I915_STATE_WARN(pll->active_mask & pipe_mask,
+                               "pll active mismatch (didn't expect pipe %c in active mask (0x%x))\n",
+                               pipe_name(crtc->pipe), pll->active_mask);
+               I915_STATE_WARN(pll->state.pipe_mask & pipe_mask,
+                               "pll enabled crtcs mismatch (found %x in enabled mask (0x%x))\n",
+                               pipe_name(crtc->pipe), pll->state.pipe_mask);
        }
 }
 

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 449470ef9f6595c0bc877987c94f76adf04095f3..0c5b7600d84775fa16f350ccd180fd102e2d5420 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1098,8 +1098,8 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 
                seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->info->name,
                           pll->info->id);
-               seq_printf(m, " crtc_mask: 0x%08x, active: 0x%x, on: %s\n",
-                          pll->state.crtc_mask, pll->active_mask, yesno(pll->on));
+               seq_printf(m, " pipe_mask: 0x%x, active: 0x%x, on: %s\n",
+                          pll->state.pipe_mask, pll->active_mask, yesno(pll->on));
                seq_printf(m, " tracked hardware state:\n");
                seq_printf(m, " dpll:    0x%08x\n", pll->state.hw_state.dpll);
                seq_printf(m, " dpll_md: 0x%08x\n",

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index ac6460962e298dc3af57e44b03d0b9a1c98d9fc0..22ee8e13b518c89f182f48b2ff4d1519bfaa2922 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -176,7 +176,7 @@ void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state)
                return;
 
        mutex_lock(&dev_priv->dpll.lock);
-       drm_WARN_ON(&dev_priv->drm, !pll->state.crtc_mask);
+       drm_WARN_ON(&dev_priv->drm, !pll->state.pipe_mask);
        if (!pll->active_mask) {
                drm_dbg(&dev_priv->drm, "setting up %s\n", pll->info->name);
                drm_WARN_ON(&dev_priv->drm, pll->on);
@@ -198,7 +198,7 @@ void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
        struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
        struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
        struct intel_shared_dpll *pll = crtc_state->shared_dpll;
-       unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
+       unsigned int pipe_mask = BIT(crtc->pipe);
        unsigned int old_mask;
 
        if (drm_WARN_ON(&dev_priv->drm, pll == NULL))
@@ -207,16 +207,16 @@ void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
        mutex_lock(&dev_priv->dpll.lock);
        old_mask = pll->active_mask;
 
-       if (drm_WARN_ON(&dev_priv->drm, !(pll->state.crtc_mask & crtc_mask)) ||
-           drm_WARN_ON(&dev_priv->drm, pll->active_mask & crtc_mask))
+       if (drm_WARN_ON(&dev_priv->drm, !(pll->state.pipe_mask & pipe_mask)) ||
+           drm_WARN_ON(&dev_priv->drm, pll->active_mask & pipe_mask))
                goto out;
 
-       pll->active_mask |= crtc_mask;
+       pll->active_mask |= pipe_mask;
 
        drm_dbg_kms(&dev_priv->drm,
-                   "enable %s (active %x, on? %d) for crtc %d\n",
+                   "enable %s (active 0x%x, on? %d) for [CRTC:%d:%s]\n",
                    pll->info->name, pll->active_mask, pll->on,
-                   crtc->base.base.id);
+                   crtc->base.base.id, crtc->base.name);
 
        if (old_mask) {
                drm_WARN_ON(&dev_priv->drm, !pll->on);
@@ -244,7 +244,7 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
        struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
        struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
        struct intel_shared_dpll *pll = crtc_state->shared_dpll;
-       unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
+       unsigned int pipe_mask = BIT(crtc->pipe);
 
        /* PCH only available on ILK+ */
        if (INTEL_GEN(dev_priv) < 5)
@@ -254,18 +254,20 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
                return;
 
        mutex_lock(&dev_priv->dpll.lock);
-       if (drm_WARN_ON(&dev_priv->drm, !(pll->active_mask & crtc_mask)))
+       if (drm_WARN(&dev_priv->drm, !(pll->active_mask & pipe_mask),
+                    "%s not used by [CRTC:%d:%s]\n", pll->info->name,
+                    crtc->base.base.id, crtc->base.name))
                goto out;
 
        drm_dbg_kms(&dev_priv->drm,
-                   "disable %s (active %x, on? %d) for crtc %d\n",
+                   "disable %s (active 0x%x, on? %d) for [CRTC:%d:%s]\n",
                    pll->info->name, pll->active_mask, pll->on,
-                   crtc->base.base.id);
+                   crtc->base.base.id, crtc->base.name);
 
        assert_shared_dpll_enabled(dev_priv, pll);
        drm_WARN_ON(&dev_priv->drm, !pll->on);
 
-       pll->active_mask &= ~crtc_mask;
+       pll->active_mask &= ~pipe_mask;
        if (pll->active_mask)
                goto out;
 
@@ -296,7 +298,7 @@ intel_find_shared_dpll(struct intel_atomic_state *state,
                pll = &dev_priv->dpll.shared_dplls[i];
 
                /* Only want to check enabled timings first */
-               if (shared_dpll[i].crtc_mask == 0) {
+               if (shared_dpll[i].pipe_mask == 0) {
                        if (!unused_pll)
                                unused_pll = pll;
                        continue;
@@ -306,10 +308,10 @@ intel_find_shared_dpll(struct intel_atomic_state *state,
                           &shared_dpll[i].hw_state,
                           sizeof(*pll_state)) == 0) {
                        drm_dbg_kms(&dev_priv->drm,
-                                   "[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, active %x)\n",
+                                   "[CRTC:%d:%s] sharing existing %s (pipe mask 0x%x, active 0x%x)\n",
                                    crtc->base.base.id, crtc->base.name,
                                    pll->info->name,
-                                   shared_dpll[i].crtc_mask,
+                                   shared_dpll[i].pipe_mask,
                                    pll->active_mask);
                        return pll;
                }
@@ -338,13 +340,13 @@ intel_reference_shared_dpll(struct intel_atomic_state *state,
 
        shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
 
-       if (shared_dpll[id].crtc_mask == 0)
+       if (shared_dpll[id].pipe_mask == 0)
                shared_dpll[id].hw_state = *pll_state;
 
        drm_dbg(&i915->drm, "using %s for pipe %c\n", pll->info->name,
                pipe_name(crtc->pipe));
 
-       shared_dpll[id].crtc_mask |= 1 << crtc->pipe;
+       shared_dpll[id].pipe_mask |= BIT(crtc->pipe);
 }
 
 static void intel_unreference_shared_dpll(struct intel_atomic_state *state,
@@ -354,7 +356,7 @@ static void intel_unreference_shared_dpll(struct intel_atomic_state *state,
        struct intel_shared_dpll_state *shared_dpll;
 
        shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
-       shared_dpll[pll->info->id].crtc_mask &= ~(1 << crtc->pipe);
+       shared_dpll[pll->info->id].pipe_mask &= ~BIT(crtc->pipe);
 }
 
 static void intel_put_dpll(struct intel_atomic_state *state,
@@ -4597,19 +4599,19 @@ static void readout_dpll_hw_state(struct drm_i915_private *i915,
                                                       POWER_DOMAIN_DPLL_DC_OFF);
        }
 
-       pll->state.crtc_mask = 0;
+       pll->state.pipe_mask = 0;
        for_each_intel_crtc(&i915->drm, crtc) {
                struct intel_crtc_state *crtc_state =
                        to_intel_crtc_state(crtc->base.state);
 
                if (crtc_state->hw.active && crtc_state->shared_dpll == pll)
-                       pll->state.crtc_mask |= 1 << crtc->pipe;
+                       pll->state.pipe_mask |= BIT(crtc->pipe);
        }
-       pll->active_mask = pll->state.crtc_mask;
+       pll->active_mask = pll->state.pipe_mask;
 
        drm_dbg_kms(&i915->drm,
-                   "%s hw state readout: crtc_mask 0x%08x, on %i\n",
-                   pll->info->name, pll->state.crtc_mask, pll->on);
+                   "%s hw state readout: pipe_mask 0x%x, on %i\n",
+                   pll->info->name, pll->state.pipe_mask, pll->on);
 }
 
 void intel_dpll_update_ref_clks(struct drm_i915_private *i915)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index 81e67639dadb4a3cf0db53af4a378d376731f6b3..7fd031a70cfd35d6734ef4a52abbc17ab7942b01 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -241,9 +241,9 @@ struct intel_dpll_hw_state {
  */
 struct intel_shared_dpll_state {
        /**
-        * @crtc_mask: mask of CRTC using this DPLL, active or not
+        * @pipe_mask: mask of pipes using this DPLL, active or not
         */
-       unsigned crtc_mask;
+       u8 pipe_mask;
 
        /**
         * @hw_state: hardware configuration for the DPLL stored in
@@ -351,9 +351,9 @@ struct intel_shared_dpll {
        struct intel_shared_dpll_state state;
 
        /**
-        * @active_mask: mask of active CRTCs (i.e. DPMS on) using this DPLL
+        * @active_mask: mask of active pipes (i.e. DPMS on) using this DPLL
         */
-       unsigned active_mask;
+       u8 active_mask;
 
        /**
         * @on: is the PLL actually active? Disabled during modeset