Re: [PATCH 66/89] drm/i915/skl: Implementation of SKL DPLL programming

Damien Lespiau
2015-05-13T15:31:56Z

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@intel.com>
> 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@lists.freedesktop.org