On Wed, May 13, 2015 at 03:54:32PM +0100, Tvrtko Ursulin wrote: > > Hi, Hi Tvrtko, > >+static bool > >+skl_ddi_pll_select(struct intel_crtc *intel_crtc, > >+ struct intel_encoder *intel_encoder, > >+ int clock) > >+{ > >+ struct intel_shared_dpll *pll; > >+ uint32_t ctrl1, cfgcr1, cfgcr2; > >+ > >+ /* > >+ * See comment in intel_dpll_hw_state to understand why we always use 0 > >+ * as the DPLL id in this function. > >+ */ > >+ > >+ ctrl1 = DPLL_CTRL1_OVERRIDE(0); > >+ > >+ if (intel_encoder->type == INTEL_OUTPUT_HDMI) { > >+ struct skl_wrpll_params wrpll_params = { 0, }; > >+ > >+ ctrl1 |= DPLL_CTRL1_HDMI_MODE(0); > >+ > >+ skl_ddi_calculate_wrpll(clock * 1000, &wrpll_params); > >+ > >+ cfgcr1 = DPLL_CFGCR1_FREQ_ENABLE | > >+ DPLL_CFGCR1_DCO_FRACTION(wrpll_params.dco_fraction) | > >+ wrpll_params.dco_integer; > >+ > >+ cfgcr2 = DPLL_CFGCR2_QDIV_RATIO(wrpll_params.qdiv_ratio) | > >+ DPLL_CFGCR2_QDIV_MODE(wrpll_params.qdiv_mode) | > >+ DPLL_CFGCR2_KDIV(wrpll_params.kdiv) | > >+ DPLL_CFGCR2_PDIV(wrpll_params.pdiv) | > >+ wrpll_params.central_freq; > >+ } else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) { > >+ struct drm_encoder *encoder = &intel_encoder->base; > >+ struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > >+ > >+ switch (intel_dp->link_bw) { > >+ case DP_LINK_BW_1_62: > >+ ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_810, 0); > >+ break; > >+ case DP_LINK_BW_2_7: > >+ ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1350, 0); > >+ break; > >+ case DP_LINK_BW_5_4: > >+ ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2700, 0); > >+ break; > >+ } > >+ > >+ cfgcr1 = cfgcr2 = 0; > >+ } else /* eDP */ > >+ return true; > > > >+ intel_crtc->config.dpll_hw_state.ctrl1 = ctrl1; > >+ intel_crtc->config.dpll_hw_state.cfgcr1 = cfgcr1; > >+ intel_crtc->config.dpll_hw_state.cfgcr2 = cfgcr2; > > The return before updating the dpll_hw_state in the eDP case interacts with: > > commit 4978cc93d9ac240b435ce60431aef24239b4c270 > Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> > Date: Tue Apr 21 17:13:21 2015 +0300 > > drm/i915: Preserve shared DPLL information in new pipe_config > > And results with a bunch of: > > [drm:check_crtc_state [i915]] *ERROR* mismatch in dpll_hw_state.ctrl1 > (expected 0x00000000, found 0x00000001) > > On boot and afterwards. > > Not sure what is the fix so maybe someone knows? I, since then, added the hint: /* * Tries to find a *shared* PLL for the CRTC and store it in * intel_crtc->ddi_pll_sel. * * For private DPLLs, compute_config() should do the selection for us. This * function should be folded into compute_config() eventually. */ The mess is because of the shared vs private PLLs. For eDP skl_edp_set_pll_config() in intel_dp.c should take care of computing the state. -- Damien _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx