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 <[email protected]>
Reviewed-by: Damien Lespiau <[email protected]> -- 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
