[Intel-gfx] [PATCH 66/89] drm/i915/skl: Implementation of SKL DPLL programming

[Damien Lespiau]

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 at 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