drm/i915: Track full cdclk state for the logical and actual cdclk frequencies

The current dev_cdclk vs. cdclk vs. atomic_cdclk_freq is quite a mess.
So here I'm introducing the "actual" and "logical" naming for our
cdclk state. "actual" is what we'll bash into the hardware and "logical"
is what everyone should use for state computaion/checking and whatnot.
We'll track both using the intel_cdclk_state as both will need other
differing parameters than just the actual cdclk frequency.

While doing that we can at the same time unify the appearance of the
.modeset_calc_cdclk() implementations a little bit.

v2: Commit dev_priv->cdclk.actual since that already has the
    new state by the time .modeset_commit_cdclk() is called.
v3: s/locical/logical/ and improve the docs a bit

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170120182205.8141-9-ville.syrjala@linux.intel.com

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index aa59b21533749bfe0fb5ddd394a3a183ba442893..323bf93c3e927eae52387e61d00aef4dc8bc82fe 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,18 +2172,26 @@ struct drm_i915_private {
        unsigned int skl_preferred_vco_freq;
        unsigned int max_cdclk_freq;
 
-       /*
-        * For reading holding any crtc lock is sufficient,
-        * for writing must hold all of them.
-        */
-       unsigned int atomic_cdclk_freq;
-
        unsigned int max_dotclk_freq;
        unsigned int rawclk_freq;
        unsigned int hpll_freq;
        unsigned int czclk_freq;
 
        struct {
+               /*
+                * The current logical cdclk state.
+                * See intel_atomic_state.cdclk.logical
+                *
+                * For reading holding any crtc lock is sufficient,
+                * for writing must hold all of them.
+                */
+               struct intel_cdclk_state logical;
+               /*
+                * The current actual cdclk state.
+                * See intel_atomic_state.cdclk.actual
+                */
+               struct intel_cdclk_state actual;
+               /* The current hardware cdclk state */
                struct intel_cdclk_state hw;
        } cdclk;
 

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 4e74e87a8676efcc34cb4b0ecb2538962fc368c1..6529a2687fdda36b53d8386ed52fd97a39b45582 100644 (file)
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1460,12 +1460,26 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
        int max_pixclk = intel_max_pixel_rate(state);
        struct intel_atomic_state *intel_state =
                to_intel_atomic_state(state);
+       int cdclk;
+
+       cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
+
+       if (cdclk > dev_priv->max_cdclk_freq) {
+               DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+                             cdclk, dev_priv->max_cdclk_freq);
+               return -EINVAL;
+       }
 
-       intel_state->cdclk = intel_state->dev_cdclk =
-               vlv_calc_cdclk(dev_priv, max_pixclk);
+       intel_state->cdclk.logical.cdclk = cdclk;
 
-       if (!intel_state->active_crtcs)
-               intel_state->dev_cdclk = vlv_calc_cdclk(dev_priv, 0);
+       if (!intel_state->active_crtcs) {
+               cdclk = vlv_calc_cdclk(dev_priv, 0);
+
+               intel_state->cdclk.actual.cdclk = cdclk;
+       } else {
+               intel_state->cdclk.actual =
+                       intel_state->cdclk.logical;
+       }
 
        return 0;
 }
@@ -1474,9 +1488,7 @@ static void vlv_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 {
        struct drm_device *dev = old_state->dev;
        struct drm_i915_private *dev_priv = to_i915(dev);
-       struct intel_atomic_state *old_intel_state =
-               to_intel_atomic_state(old_state);
-       unsigned int req_cdclk = old_intel_state->dev_cdclk;
+       unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
 
        /*
         * FIXME: We can end up here with all power domains off, yet
@@ -1518,9 +1530,16 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
                return -EINVAL;
        }
 
-       intel_state->cdclk = intel_state->dev_cdclk = cdclk;
-       if (!intel_state->active_crtcs)
-               intel_state->dev_cdclk = bdw_calc_cdclk(0);
+       intel_state->cdclk.logical.cdclk = cdclk;
+
+       if (!intel_state->active_crtcs) {
+               cdclk = bdw_calc_cdclk(0);
+
+               intel_state->cdclk.actual.cdclk = cdclk;
+       } else {
+               intel_state->cdclk.actual =
+                       intel_state->cdclk.logical;
+       }
 
        return 0;
 }
@@ -1528,9 +1547,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 static void bdw_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 {
        struct drm_device *dev = old_state->dev;
-       struct intel_atomic_state *old_intel_state =
-               to_intel_atomic_state(old_state);
-       unsigned int req_cdclk = old_intel_state->dev_cdclk;
+       unsigned int req_cdclk = to_i915(dev)->cdclk.actual.cdclk;
 
        bdw_set_cdclk(dev, req_cdclk);
 }
@@ -1540,8 +1557,11 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
        struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
        struct drm_i915_private *dev_priv = to_i915(state->dev);
        const int max_pixclk = intel_max_pixel_rate(state);
-       int vco = intel_state->cdclk_pll_vco;
-       int cdclk;
+       int cdclk, vco;
+
+       vco = intel_state->cdclk.logical.vco;
+       if (!vco)
+               vco = dev_priv->skl_preferred_vco_freq;
 
        /*
         * FIXME should also account for plane ratio
@@ -1549,19 +1569,24 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
         */
        cdclk = skl_calc_cdclk(max_pixclk, vco);
 
-       /*
-        * FIXME move the cdclk caclulation to
-        * compute_config() so we can fail gracegully.
-        */
        if (cdclk > dev_priv->max_cdclk_freq) {
-               DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-                         cdclk, dev_priv->max_cdclk_freq);
-               cdclk = dev_priv->max_cdclk_freq;
+               DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+                             cdclk, dev_priv->max_cdclk_freq);
+               return -EINVAL;
        }
 
-       intel_state->cdclk = intel_state->dev_cdclk = cdclk;
-       if (!intel_state->active_crtcs)
-               intel_state->dev_cdclk = skl_calc_cdclk(0, vco);
+       intel_state->cdclk.logical.vco = vco;
+       intel_state->cdclk.logical.cdclk = cdclk;
+
+       if (!intel_state->active_crtcs) {
+               cdclk = skl_calc_cdclk(0, vco);
+
+               intel_state->cdclk.actual.vco = vco;
+               intel_state->cdclk.actual.cdclk = cdclk;
+       } else {
+               intel_state->cdclk.actual =
+                       intel_state->cdclk.logical;
+       }
 
        return 0;
 }
@@ -1569,10 +1594,8 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 {
        struct drm_i915_private *dev_priv = to_i915(old_state->dev);
-       struct intel_atomic_state *intel_state =
-               to_intel_atomic_state(old_state);
-       unsigned int req_cdclk = intel_state->dev_cdclk;
-       unsigned int req_vco = intel_state->cdclk_pll_vco;
+       unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
+       unsigned int req_vco = dev_priv->cdclk.actual.vco;
 
        skl_set_cdclk(dev_priv, req_cdclk, req_vco);
 }
@@ -1583,22 +1606,39 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
        int max_pixclk = intel_max_pixel_rate(state);
        struct intel_atomic_state *intel_state =
                to_intel_atomic_state(state);
-       int cdclk;
+       int cdclk, vco;
 
-       if (IS_GEMINILAKE(dev_priv))
+       if (IS_GEMINILAKE(dev_priv)) {
                cdclk = glk_calc_cdclk(max_pixclk);
-       else
+               vco = glk_de_pll_vco(dev_priv, cdclk);
+       } else {
                cdclk = bxt_calc_cdclk(max_pixclk);
+               vco = bxt_de_pll_vco(dev_priv, cdclk);
+       }
+
+       if (cdclk > dev_priv->max_cdclk_freq) {
+               DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+                             cdclk, dev_priv->max_cdclk_freq);
+               return -EINVAL;
+       }
 
-       intel_state->cdclk = intel_state->dev_cdclk = cdclk;
+       intel_state->cdclk.logical.vco = vco;
+       intel_state->cdclk.logical.cdclk = cdclk;
 
        if (!intel_state->active_crtcs) {
-               if (IS_GEMINILAKE(dev_priv))
+               if (IS_GEMINILAKE(dev_priv)) {
                        cdclk = glk_calc_cdclk(0);
-               else
+                       vco = glk_de_pll_vco(dev_priv, cdclk);
+               } else {
                        cdclk = bxt_calc_cdclk(0);
+                       vco = bxt_de_pll_vco(dev_priv, cdclk);
+               }
 
-               intel_state->dev_cdclk = cdclk;
+               intel_state->cdclk.actual.vco = vco;
+               intel_state->cdclk.actual.cdclk = cdclk;
+       } else {
+               intel_state->cdclk.actual =
+                       intel_state->cdclk.logical;
        }
 
        return 0;
@@ -1607,15 +1647,8 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 {
        struct drm_i915_private *dev_priv = to_i915(old_state->dev);
-       struct intel_atomic_state *old_intel_state =
-               to_intel_atomic_state(old_state);
-       unsigned int req_cdclk = old_intel_state->dev_cdclk;
-       unsigned int req_vco;
-
-       if (IS_GEMINILAKE(dev_priv))
-               req_vco = glk_de_pll_vco(dev_priv, req_cdclk);
-       else
-               req_vco = bxt_de_pll_vco(dev_priv, req_cdclk);
+       unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
+       unsigned int req_vco = dev_priv->cdclk.actual.vco;
 
        bxt_set_cdclk(dev_priv, req_cdclk, req_vco);
 }

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 691db36d8388f43789a49a749c88f307e2a42037..c0ad2ddaed9b067a85ef324dbdeeb26bf828d6b7 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12393,6 +12393,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 
        intel_state->modeset = true;
        intel_state->active_crtcs = dev_priv->active_crtcs;
+       intel_state->cdclk.logical = dev_priv->cdclk.logical;
+       intel_state->cdclk.actual = dev_priv->cdclk.actual;
 
        for_each_crtc_in_state(state, crtc, crtc_state, i) {
                if (crtc_state->active)
@@ -12412,38 +12414,35 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
         * adjusted_mode bits in the crtc directly.
         */
        if (dev_priv->display.modeset_calc_cdclk) {
-               if (!intel_state->cdclk_pll_vco)
-                       intel_state->cdclk_pll_vco = dev_priv->cdclk.hw.vco;
-               if (!intel_state->cdclk_pll_vco)
-                       intel_state->cdclk_pll_vco = dev_priv->skl_preferred_vco_freq;
-
                ret = dev_priv->display.modeset_calc_cdclk(state);
                if (ret < 0)
                        return ret;
 
                /*
-                * Writes to dev_priv->atomic_cdclk_freq must protected by
+                * Writes to dev_priv->cdclk.logical must protected by
                 * holding all the crtc locks, even if we don't end up
                 * touching the hardware
                 */
-               if (intel_state->cdclk != dev_priv->atomic_cdclk_freq) {
+               if (!intel_cdclk_state_compare(&dev_priv->cdclk.logical,
+                                              &intel_state->cdclk.logical)) {
                        ret = intel_lock_all_pipes(state);
                        if (ret < 0)
                                return ret;
                }
 
                /* All pipes must be switched off while we change the cdclk. */
-               if (intel_state->dev_cdclk != dev_priv->cdclk.hw.cdclk ||
-                   intel_state->cdclk_pll_vco != dev_priv->cdclk.hw.vco) {
+               if (!intel_cdclk_state_compare(&dev_priv->cdclk.actual,
+                                              &intel_state->cdclk.actual)) {
                        ret = intel_modeset_all_pipes(state);
                        if (ret < 0)
                                return ret;
                }
 
-               DRM_DEBUG_KMS("New cdclk calculated to be atomic %u, actual %u\n",
-                             intel_state->cdclk, intel_state->dev_cdclk);
+               DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
+                             intel_state->cdclk.logical.cdclk,
+                             intel_state->cdclk.actual.cdclk);
        } else {
-               to_intel_atomic_state(state)->cdclk = dev_priv->atomic_cdclk_freq;
+               to_intel_atomic_state(state)->cdclk.logical = dev_priv->cdclk.logical;
        }
 
        intel_modeset_clear_plls(state);
@@ -12546,7 +12545,7 @@ static int intel_atomic_check(struct drm_device *dev,
                if (ret)
                        return ret;
        } else {
-               intel_state->cdclk = dev_priv->atomic_cdclk_freq;
+               intel_state->cdclk.logical = dev_priv->cdclk.logical;
        }
 
        ret = drm_atomic_helper_check_planes(dev, state);
@@ -12869,8 +12868,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
                drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
 
                if (dev_priv->display.modeset_commit_cdclk &&
-                   (intel_state->dev_cdclk != dev_priv->cdclk.hw.cdclk ||
-                    intel_state->cdclk_pll_vco != dev_priv->cdclk.hw.vco))
+                   !intel_cdclk_state_compare(&dev_priv->cdclk.hw,
+                                              &dev_priv->cdclk.actual))
                        dev_priv->display.modeset_commit_cdclk(state);
 
                /*
@@ -13059,7 +13058,8 @@ static int intel_atomic_commit(struct drm_device *dev,
                memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
                       sizeof(intel_state->min_pixclk));
                dev_priv->active_crtcs = intel_state->active_crtcs;
-               dev_priv->atomic_cdclk_freq = intel_state->cdclk;
+               dev_priv->cdclk.logical = intel_state->cdclk.logical;
+               dev_priv->cdclk.actual = intel_state->cdclk.actual;
        }
 
        drm_atomic_state_get(state);
@@ -13297,7 +13297,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
                return DRM_PLANE_HELPER_NO_SCALING;
 
        crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
-       cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk;
+       cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
 
        if (WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock))
                return DRM_PLANE_HELPER_NO_SCALING;
@@ -14854,8 +14854,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
        struct drm_i915_private *dev_priv = to_i915(dev);
 
        intel_update_cdclk(dev_priv);
-
-       dev_priv->atomic_cdclk_freq = dev_priv->cdclk.hw.cdclk;
+       dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
 
        intel_init_clock_gating(dev_priv);
 }
@@ -15031,7 +15030,7 @@ int intel_modeset_init(struct drm_device *dev)
 
        intel_update_czclk(dev_priv);
        intel_update_cdclk(dev_priv);
-       dev_priv->atomic_cdclk_freq = dev_priv->cdclk.hw.cdclk;
+       dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
 
        intel_shared_dpll_init(dev);
 

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f8c23fed4a2cd17663bb4b68ab8ac35c0aa80ac5..0f14e97e519b863222a1c0e66651e341b245567b 100644 (file)
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1785,7 +1785,7 @@ found:
                        break;
                }
 
-               to_intel_atomic_state(pipe_config->base.state)->cdclk_pll_vco = vco;
+               to_intel_atomic_state(pipe_config->base.state)->cdclk.logical.vco = vco;
        }
 
        if (!HAS_DDI(dev_priv))

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ba0728ccff759d6cdf188cb14d0744f4d2357d47..97d848197ff33b6474f8d59c2957a849b3eb8edf 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -333,13 +333,20 @@ struct dpll {
 struct intel_atomic_state {
        struct drm_atomic_state base;
 
-       unsigned int cdclk;
-
-       /*
-        * Calculated device cdclk, can be different from cdclk
-        * only when all crtc's are DPMS off.
-        */
-       unsigned int dev_cdclk;
+       struct {
+               /*
+                * Logical state of cdclk (used for all scaling, watermark,
+                * etc. calculations and checks). This is computed as if all
+                * enabled crtcs were active.
+                */
+               struct intel_cdclk_state logical;
+
+               /*
+                * Actual state of cdclk, can be different from the logical
+                * state only when all crtc's are DPMS off.
+                */
+               struct intel_cdclk_state actual;
+       } cdclk;
 
        bool dpll_set, modeset;
 
@@ -356,9 +363,6 @@ struct intel_atomic_state {
        unsigned int active_crtcs;
        unsigned int min_pixclk[I915_MAX_PIPES];
 
-       /* SKL/KBL Only */
-       unsigned int cdclk_pll_vco;
-
        struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
 
        /*

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 79cbe5736ed1e514d0e4575bd17ecd6633482deb..b1d07e63ff8b017c4e1797df895ad1b7fae9881d 100644 (file)
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2108,7 +2108,7 @@ hsw_compute_linetime_wm(const struct intel_crtc_state *cstate)
                return 0;
        if (WARN_ON(adjusted_mode->crtc_clock == 0))
                return 0;
-       if (WARN_ON(intel_state->cdclk == 0))
+       if (WARN_ON(intel_state->cdclk.logical.cdclk == 0))
                return 0;
 
        /* The WM are computed with base on how long it takes to fill a single
@@ -2117,7 +2117,7 @@ hsw_compute_linetime_wm(const struct intel_crtc_state *cstate)
        linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
                                     adjusted_mode->crtc_clock);
        ips_linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
-                                        intel_state->cdclk);
+                                        intel_state->cdclk.logical.cdclk);
 
        return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
               PIPE_WM_LINETIME_TIME(linetime);