On Wed, Jun 05, 2013 at 01:34:16PM +0200, Daniel Vetter wrote:
> Currently still with an empty register state, this will follow in a
> next step. This one here just creates the new vfunc and uses it for
> cross-checking, initial state takeover and the dpll assert function.
>
> And add a FIXME for the ddi pll readout code, which still needs to be
> converted over.
>
> v2:
> - Add some hw state readout debug output.
> - Also cross check the enabled crtc counting.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 ++++
> drivers/gpu/drm/i915/intel_display.c | 77 +++++++++++++++++++++++++++++++++---
> 2 files changed, 79 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8e61128..f23b033 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -143,6 +143,9 @@ enum intel_dpll_id {
> };
> #define I915_NUM_PLLS 2
>
> +struct intel_dpll_hw_state {
> +};
> +
> struct intel_shared_dpll {
> int refcount; /* count of number of CRTCs sharing this PLL */
> int active; /* count of number of active CRTCs (i.e. DPMS on) */
> @@ -150,10 +153,14 @@ struct intel_shared_dpll {
> const char *name;
> /* should match the index in the dev_priv->shared_dplls array */
> enum intel_dpll_id id;
> + struct intel_dpll_hw_state hw_state;
> void (*enable)(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll);
> void (*disable)(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll);
> + bool (*get_hw_state)(struct drm_i915_private *dev_priv,
> + struct intel_shared_dpll *pll,
> + struct intel_dpll_hw_state *hw_state);
> };
>
> /* Used by dp and fdi links */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ea4b7a6..998ba5c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -907,8 +907,8 @@ static void assert_shared_dpll(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll,
> bool state)
> {
> - u32 val;
> bool cur_state;
> + struct intel_dpll_hw_state hw_state;
>
> if (HAS_PCH_LPT(dev_priv->dev)) {
> DRM_DEBUG_DRIVER("LPT detected: skipping PCH PLL test\n");
> @@ -919,11 +919,10 @@ static void assert_shared_dpll(struct drm_i915_private *dev_priv,
> "asserting DPLL %s with no DPLL\n", state_string(state)))
> return;
>
> - val = I915_READ(PCH_DPLL(pll->id));
> - cur_state = !!(val & DPLL_VCO_ENABLE);
> + cur_state = pll->get_hw_state(dev_priv, pll, &hw_state);
> WARN(cur_state != state,
> - "%s assertion failure (expected %s, current %s), val=%08x\n",
> - pll->name, state_string(state), state_string(cur_state), val);
> + "%s assertion failure (expected %s, current %s)\n",
> + pll->name, state_string(state), state_string(cur_state));
> }
> #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
> #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
> @@ -8071,6 +8070,8 @@ intel_modeset_check_state(struct drm_device *dev)
> struct intel_encoder *encoder;
> struct intel_connector *connector;
> struct intel_crtc_config pipe_config;
> + struct intel_dpll_hw_state dpll_hw_state;
> + int i;
>
> list_for_each_entry(connector, &dev->mode_config.connector_list,
> base.head) {
> @@ -8183,6 +8184,41 @@ intel_modeset_check_state(struct drm_device *dev)
> "[sw state]");
> }
> }
> +
> + for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
> + int enabled_crtcs = 0, active_crtcs = 0;
> + bool active;
> +
> + memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
> +
> + DRM_DEBUG_KMS("%s\n", pll->name);
> +
> + active = pll->get_hw_state(dev_priv, pll, &dpll_hw_state);
> +
> + WARN(pll->active > pll->refcount,
> + "more active pll users than references: %i vs %i\n",
> + pll->active, pll->refcount);
> + WARN(pll->active && !pll->on,
> + "pll in active use but not on in sw tracking\n");
> + WARN(pll->on != active,
> + "pll on state mismatch (expected %i, found %i)\n",
> + pll->on, active);
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> + base.head) {
> + if (crtc->base.enabled && intel_crtc_to_shared_dpll(crtc) == pll)
> + enabled_crtcs++;
> + if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll)
> + active_crtcs++;
> + }
> + WARN(pll->active != active_crtcs,
> + "pll active crtcs mismatch (expected %i, found %i)\n",
> + pll->active, active_crtcs);
> + WARN(pll->refcount != enabled_crtcs,
> + "pll enabled crtcs mismatch (expected %i, found %i)\n",
> + pll->refcount, enabled_crtcs);
> + }
> }
>
> static int __intel_set_mode(struct drm_crtc *crtc,
> @@ -8621,6 +8657,17 @@ static void intel_cpu_pll_init(struct drm_device *dev)
> intel_ddi_pll_init(dev);
> }
>
> +static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
> + struct intel_shared_dpll *pll,
> + struct intel_dpll_hw_state *hw_state)
> +{
> + uint32_t val;
> +
> + val = I915_READ(PCH_DPLL(pll->id));
> +
> + return val & DPLL_VCO_ENABLE;
> +}
> +
Don't we want !!(val & DPLL_VCO_ENABLE) here? we're comparing this to 0
and 1.
> static void ibx_pch_dpll_enable(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll)
> {
> @@ -8675,6 +8722,8 @@ static void ibx_pch_dpll_init(struct drm_device *dev)
> dev_priv->shared_dplls[i].name = ibx_pch_dpll_names[i];
> dev_priv->shared_dplls[i].enable = ibx_pch_dpll_enable;
> dev_priv->shared_dplls[i].disable = ibx_pch_dpll_disable;
> + dev_priv->shared_dplls[i].get_hw_state =
> + ibx_pch_dpll_get_hw_state;
> }
> }
>
> @@ -9592,6 +9641,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> struct intel_crtc *crtc;
> struct intel_encoder *encoder;
> struct intel_connector *connector;
> + int i;
>
> list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> base.head) {
> @@ -9607,9 +9657,26 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> crtc->active ? "enabled" : "disabled");
> }
>
> + /* FIXME: Smash this into the new shared dpll infrastructure. */
> if (HAS_DDI(dev))
> intel_ddi_setup_hw_pll_state(dev);
>
> + for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
> +
> + pll->on = pll->get_hw_state(dev_priv, pll, &pll->hw_state);
> + pll->active = 0;
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> + base.head) {
> + if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll)
> + pll->active++;
> + }
> + pll->refcount = pll->active;
> +
> + DRM_DEBUG_KMS("%s hw state readout: refcount %i\n",
> + pll->name, pll->refcount);
> + }
> +
> list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> base.head) {
> pipe = 0;
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
