On Wed, Jun 05, 2013 at 01:34:20PM +0200, Daniel Vetter wrote:
> Just the plumbing, all the modeset and enable code has not yet been
> switched over to use the new state. It seems to be decently broken
> anyway, at least wrt to handling of the special pixel mutliplier
> enabling sequence. Follow-up patches will clean up that mess.
>
> Another missing piece is more careful handling (and fixup) of the fp1
> alternate divisor state. The BIOS most likely doesn't bother to
> program that one to what we expect. So we need to be more careful with
> comparing that state, both for cross checking but also when checking
> for dpll sharing when acquiring shared dpll. Otherwise fastboot will
> deny a few shared dpll configurations which would otherwise work.
>
> v2: We need to memcpy the pipe config dpll hw state into the pll, for
> otherwise the cross-check code will get angry.
>
> v3: Don't forget to read the pch pll state in the crtc get_pipe_config
> function for ibx/ilk platforms.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau at intel.com>
--
Damien
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> 3 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f23b033..4dc94ed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -144,6 +144,9 @@ enum intel_dpll_id {
> #define I915_NUM_PLLS 2
>
> struct intel_dpll_hw_state {
> + uint32_t dpll;
> + uint32_t fp0;
> + uint32_t fp1;
> };
>
> struct intel_shared_dpll {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 388ac54..a30e27a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3055,7 +3055,11 @@ found:
> crtc->config.shared_dpll = i;
> DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
> pipe_name(crtc->pipe));
> +
> if (pll->active == 0) {
> + memcpy(&pll->hw_state, &crtc->config.dpll_hw_state,
> + sizeof(pll->hw_state));
> +
> DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
> WARN_ON(pll->on);
> assert_shared_dpll_disabled(dev_priv, pll);
> @@ -5659,6 +5663,13 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> &fp, &reduced_clock,
> has_reduced_clock ? &fp2 : NULL);
>
> + intel_crtc->config.dpll_hw_state.dpll = dpll | DPLL_VCO_ENABLE;
> + intel_crtc->config.dpll_hw_state.fp0 = fp;
> + if (has_reduced_clock)
> + intel_crtc->config.dpll_hw_state.fp1 = fp2;
> + else
> + intel_crtc->config.dpll_hw_state.fp1 = fp;
> +
> pll = intel_get_shared_dpll(intel_crtc, dpll, fp);
> if (pll == NULL) {
> DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> @@ -5778,6 +5789,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> return false;
>
> if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> + struct intel_shared_dpll *pll;
> +
> pipe_config->has_pch_encoder = true;
>
> tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
> @@ -5799,6 +5812,11 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> else
> pipe_config->shared_dpll = DPLL_ID_PCH_PLL_A;
> }
> +
> + pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
> +
> + WARN_ON(!pll->get_hw_state(dev_priv, pll,
> + &pipe_config->dpll_hw_state));
> } else {
> pipe_config->pixel_multiplier = 1;
> }
> @@ -7987,6 +8005,15 @@ intel_pipe_config_compare(struct drm_device *dev,
> struct intel_crtc_config *current_config,
> struct intel_crtc_config *pipe_config)
> {
> +#define PIPE_CONF_CHECK_X(name) \
> + if (current_config->name != pipe_config->name) { \
> + DRM_ERROR("mismatch in " #name " " \
> + "(expected 0x%08x, found 0x%08x)\n", \
> + current_config->name, \
> + pipe_config->name); \
> + return false; \
> + }
> +
> #define PIPE_CONF_CHECK_I(name) \
> if (current_config->name != pipe_config->name) { \
> DRM_ERROR("mismatch in " #name " " \
> @@ -8055,7 +8082,11 @@ intel_pipe_config_compare(struct drm_device *dev,
> PIPE_CONF_CHECK_I(ips_enabled);
>
> PIPE_CONF_CHECK_I(shared_dpll);
> + PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
> + PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
> + PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
>
> +#undef PIPE_CONF_CHECK_X
> #undef PIPE_CONF_CHECK_I
> #undef PIPE_CONF_CHECK_FLAGS
>
> @@ -8237,6 +8268,10 @@ check_shared_dpll_state(struct drm_device *dev)
> WARN(pll->refcount != enabled_crtcs,
> "pll enabled crtcs mismatch (expected %i, found %i)\n",
> pll->refcount, enabled_crtcs);
> +
> + WARN(pll->on && memcmp(&pll->hw_state, &dpll_hw_state,
> + sizeof(dpll_hw_state)),
> + "pll hw state mismatch\n");
> }
> }
>
> @@ -8692,6 +8727,9 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
> uint32_t val;
>
> val = I915_READ(PCH_DPLL(pll->id));
> + hw_state->dpll = val;
> + hw_state->fp0 = I915_READ(PCH_FP0(pll->id));
> + hw_state->fp1 = I915_READ(PCH_FP1(pll->id));
>
> return val & DPLL_VCO_ENABLE;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e0e5d55..6f28375 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -244,6 +244,9 @@ struct intel_crtc_config {
> /* Selected dpll when shared or DPLL_ID_PRIVATE. */
> enum intel_dpll_id shared_dpll;
>
> + /* Actual register state of the dpll, for shared dpll cross-checking. */
> + struct intel_dpll_hw_state dpll_hw_state;
> +
> int pipe_bpp;
> struct intel_link_m_n dp_m_n;
>
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
