On Wed, Jun 05, 2013 at 01:34:10PM +0200, Daniel Vetter wrote:
> Well, the first step of a long road at least, it only reads out
> the pipe -> shared dpll association thus far. Other state which needs
> to follow:
>
> - hw state of the dpll (on/off + dpll registers). Currently we just
> read that out from the hw state, but that doesn't work too well when
> the dpll is in use, but not yet fully enabled. We get away since
> most likely it already has been enabled and so the correct state is
> left behind in the registers. But that doesn't hold for atomic
> modesets when we want to enable all pipes at once.
>
> - Refcount reconstruction for each dpll.
>
> - Cross-checking of all the above. For that we need to keep the dpll
> register state both in the pipe and in the shared_dpll struct, so
> that we can check that every pipe is still connected to a correctly
> configured dpll.
>
> Note that since the refcount resconstruction isn't done yet this will
> spill a few WARNs at boot-up while trying to disable pch plls which
> have bogus refcounts. But since there's still a pile of refactoring to
> do I'd like to lock down the state handling as soon as possible hence
> decided against reordering the patches to quiet these WARNs - after
> all the issues they're complaining about have existed since forever,
> as Jesse can testify by having pch pll states blow up consistently in
> his fastboot patches ...
>
> v2: We need to preserve the old shared_dpll since currently the
> shared dpll refcount dropping/getting is done in ->mode_set. With
> the usual pipe_config infrastructure the old dpll id is already lost
> at that point, hence preserve it in the new config.
Ah so here is the missing link. The correct place for this would appear
to be patch 6.
> v3: Rebase on top of the ips patch from Paulo.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++++++------
> 1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a44c43c..20a933f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4020,12 +4020,11 @@ static void hsw_compute_ips_config(struct intel_crtc *crtc,
> pipe_config->pipe_bpp == 24;
> }
>
> -static int intel_crtc_compute_config(struct drm_crtc *crtc,
> +static int intel_crtc_compute_config(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> if (HAS_PCH_SPLIT(dev)) {
> /* FDI link clock is fixed at 2.7G */
> @@ -4056,10 +4055,16 @@ static int intel_crtc_compute_config(struct drm_crtc *crtc,
> }
>
> if (IS_HASWELL(dev))
> - hsw_compute_ips_config(intel_crtc, pipe_config);
> + hsw_compute_ips_config(crtc, pipe_config);
> +
> + /* XXX: PCH clock sharing is done in ->mode_set, so make sure the old
> + * clock survives for now. */
> + if (pipe_config->has_pch_encoder &&
> + (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)))
> + pipe_config->shared_dpll = crtc->config.shared_dpll;
>
> if (pipe_config->has_pch_encoder)
> - return ironlake_fdi_compute_config(intel_crtc, pipe_config);
> + return ironlake_fdi_compute_config(crtc, pipe_config);
>
> return 0;
> }
> @@ -4934,6 +4939,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> uint32_t tmp;
>
> pipe_config->cpu_transcoder = crtc->pipe;
> + pipe_config->shared_dpll = DPLL_ID_PRIVATE;
>
> tmp = I915_READ(PIPECONF(crtc->pipe));
> if (!(tmp & PIPECONF_ENABLE))
> @@ -5810,6 +5816,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> uint32_t tmp;
>
> pipe_config->cpu_transcoder = crtc->pipe;
> + pipe_config->shared_dpll = DPLL_ID_PRIVATE;
>
> tmp = I915_READ(PIPECONF(crtc->pipe));
> if (!(tmp & PIPECONF_ENABLE))
> @@ -5827,6 +5834,16 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> /* XXX: Can't properly read out the pch dpll pixel multiplier
> * since we don't have state tracking for pch clocks yet. */
> pipe_config->pixel_multiplier = 1;
> +
> + if (HAS_PCH_IBX(dev_priv->dev)) {
> + pipe_config->shared_dpll = crtc->pipe;
Slightly magic, but all right.
> + } else {
> + tmp = I915_READ(PCH_DPLL_SEL);
> + if (tmp & TRANS_DPLLB_SEL(crtc->pipe))
> + pipe_config->shared_dpll = DPLL_ID_PCH_PLL_B;
> + else
> + pipe_config->shared_dpll = DPLL_ID_PCH_PLL_A;
> + }
> } else {
> pipe_config->pixel_multiplier = 1;
> }
> @@ -5907,6 +5924,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> uint32_t tmp;
>
> pipe_config->cpu_transcoder = crtc->pipe;
> + pipe_config->shared_dpll = DPLL_ID_PRIVATE;
> +
> tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> if (tmp & TRANS_DDI_FUNC_ENABLE) {
> enum pipe trans_edp_pipe;
> @@ -7768,6 +7787,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> drm_mode_copy(&pipe_config->adjusted_mode, mode);
> drm_mode_copy(&pipe_config->requested_mode, mode);
> pipe_config->cpu_transcoder = to_intel_crtc(crtc)->pipe;
> + pipe_config->shared_dpll = DPLL_ID_PRIVATE;
Do we want to put this here, or should we have some DPLL_ID_INVALID, so
that we'd always catch cases where we forgot to really think about which
PLL should be used?
>
> /* Compute a starting value for pipe_config->pipe_bpp taking the source
> * plane pixel format and any sink constraints into account. Returns the
> @@ -7816,7 +7836,7 @@ encoder_retry:
> if (!pipe_config->port_clock)
> pipe_config->port_clock = pipe_config->adjusted_mode.clock;
>
> - ret = intel_crtc_compute_config(crtc, pipe_config);
> + ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config);
> if (ret < 0) {
> DRM_DEBUG_KMS("CRTC fixup failed\n");
> goto fail;
> @@ -8079,6 +8099,8 @@ intel_pipe_config_compare(struct drm_device *dev,
>
> PIPE_CONF_CHECK_I(ips_enabled);
>
> + PIPE_CONF_CHECK_I(shared_dpll);
> +
> #undef PIPE_CONF_CHECK_I
> #undef PIPE_CONF_CHECK_FLAGS
>
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrj?l?
Intel OTC
