From bb0f4aab0e7677e91cde443fecc18e71fbb85038 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 20 Jan 2017 20:21:59 +0200 Subject: [PATCH] drm/i915: Track full cdclk state for the logical and actual cdclk frequencies MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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ä Reviewed-by: Ander Conselvan de Oliveira Link: http://patchwork.freedesktop.org/patch/msgid/20170120182205.8141-9-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 20 +++-- drivers/gpu/drm/i915/intel_cdclk.c | 123 +++++++++++++++++---------- drivers/gpu/drm/i915/intel_display.c | 39 +++++---- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 24 +++--- drivers/gpu/drm/i915/intel_pm.c | 4 +- 6 files changed, 128 insertions(+), 84 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index aa59b2153374..323bf93c3e92 100644 --- 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 4e74e87a8676..6529a2687fdd 100644 --- 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 691db36d8388..c0ad2ddaed9b 100644 --- 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 f8c23fed4a2c..0f14e97e519b 100644 --- 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 ba0728ccff75..97d848197ff3 100644 --- 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 79cbe5736ed1..b1d07e63ff8b 100644 --- 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); -- 2.20.1