On Wed, Sep 12, 2012 at 10:06:35AM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni at intel.com> > > This is all the code responsible for setting the PCH PLLs. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 260 ++++++++++++++++++++-------------- > 1 file changed, 153 insertions(+), 107 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index dc4132a..7ffbcfd 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4884,6 +4884,143 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc, > return true; > } > > +static bool ironlake_pch_pll_set(struct drm_crtc *crtc, > + struct drm_display_mode *adjusted_mode, > + intel_clock_t *clock, > + bool has_reduced_clock, > + intel_clock_t *reduced_clock) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_encoder *intel_encoder; > + int num_connectors = 0; > + u32 dpll, fp = 0, fp2 = 0; > + int factor, pixel_multiplier; > + struct intel_pch_pll *pll; > + bool is_lvds = false, is_sdvo = false, is_tv = false; > + bool is_dp = false, is_cpu_edp = false; > + > + for_each_encoder_on_crtc(dev, crtc, intel_encoder) { > + switch (intel_encoder->type) { > + case INTEL_OUTPUT_LVDS: > + is_lvds = true; > + break; > + case INTEL_OUTPUT_SDVO: > + case INTEL_OUTPUT_HDMI: > + is_sdvo = true; > + if (intel_encoder->needs_tv_clock) > + is_tv = true; > + break; > + case INTEL_OUTPUT_TVOUT: > + is_tv = true; > + break; > + case INTEL_OUTPUT_DISPLAYPORT: > + is_dp = true; > + break; > + case INTEL_OUTPUT_EDP: > + is_dp = true; > + if (!intel_encoder_is_pch_edp(&intel_encoder->base)) > + is_cpu_edp = true; > + break; > + } > + > + num_connectors++; > + } > + > + fp = clock->n << 16 | clock->m1 << 8 | clock->m2; > + if (has_reduced_clock) > + fp2 = reduced_clock->n << 16 | reduced_clock->m1 << 8 | > + reduced_clock->m2; > + > + /* Enable autotuning of the PLL clock (if permissible) */ > + factor = 21; > + if (is_lvds) { > + if ((intel_panel_use_ssc(dev_priv) && > + dev_priv->lvds_ssc_freq == 100) || > + (I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP) > + factor = 25; > + } else if (is_sdvo && is_tv) > + factor = 20; > + > + if (clock->m < factor * clock->n) > + fp |= FP_CB_TUNE; > + > + dpll = 0; > + > + if (is_lvds) > + dpll |= DPLLB_MODE_LVDS; > + else > + dpll |= DPLLB_MODE_DAC_SERIAL; > + if (is_sdvo) { > + pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode); > + if (pixel_multiplier > 1) { > + dpll |= (pixel_multiplier - 1) << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT; > + } > + dpll |= DPLL_DVO_HIGH_SPEED; > + } > + if (is_dp && !is_cpu_edp) > + dpll |= DPLL_DVO_HIGH_SPEED; > + > + /* compute bitmask from p1 value */ > + dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT; > + /* also FPA1 */ > + dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT; > + > + switch (clock->p2) { > + case 5: > + dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_5; > + break; > + case 7: > + dpll |= DPLLB_LVDS_P2_CLOCK_DIV_7; > + break; > + case 10: > + dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_10; > + break; > + case 14: > + dpll |= DPLLB_LVDS_P2_CLOCK_DIV_14; > + break; > + } > + > + if (is_sdvo && is_tv) > + dpll |= PLL_REF_INPUT_TVCLKINBC; > + else if (is_tv) > + /* XXX: just matching BIOS for now */ > + /* dpll |= PLL_REF_INPUT_TVCLKINBC; */ > + dpll |= 3; > + else if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > + dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN; > + else > + dpll |= PLL_REF_INPUT_DREFCLK; > + > + pll = intel_get_pch_pll(intel_crtc, dpll, fp); > + if (!pll) > + return false; > + > + I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll); > + > + /* Wait for the clocks to stabilize. */ > + POSTING_READ(intel_crtc->pch_pll->pll_reg); > + udelay(150); > + > + /* The pixel multiplier can only be updated once the > + * DPLL is enabled and the clocks are stable. > + * > + * So write it again. > + */ > + I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll); > + > + intel_crtc->lowfreq_avail = false; > + if (is_lvds && has_reduced_clock && i915_powersave) { > + I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp2); > + intel_crtc->lowfreq_avail = true; > + } else { > + I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp); > + } > + > + return true; > +} > + > static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > struct drm_display_mode *mode, > struct drm_display_mode *adjusted_mode, > @@ -4897,11 +5034,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > int plane = intel_crtc->plane; > int num_connectors = 0; > intel_clock_t clock, reduced_clock; > - u32 dpll, fp = 0, fp2 = 0; > bool ok, has_reduced_clock = false, is_sdvo = false; > bool is_crt = false, is_lvds = false, is_tv = false, is_dp = false; > struct intel_encoder *encoder; > - int ret, factor; > + int ret; > unsigned int pipe_bpp; > bool dither; > bool is_cpu_edp = false, is_pch_edp = false; > @@ -4961,90 +5097,25 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > } > intel_crtc->bpp = pipe_bpp; > > - fp = clock.n << 16 | clock.m1 << 8 | clock.m2; > - if (has_reduced_clock) > - fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 | > - reduced_clock.m2; > - > - /* Enable autotuning of the PLL clock (if permissible) */ > - factor = 21; > - if (is_lvds) { > - if ((intel_panel_use_ssc(dev_priv) && > - dev_priv->lvds_ssc_freq == 100) || > - (I915_READ(PCH_LVDS) & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP) > - factor = 25; > - } else if (is_sdvo && is_tv) > - factor = 20; > - > - if (clock.m < factor * clock.n) > - fp |= FP_CB_TUNE; > - > - dpll = 0; > - > - if (is_lvds) > - dpll |= DPLLB_MODE_LVDS; > - else > - dpll |= DPLLB_MODE_DAC_SERIAL; > - if (is_sdvo) { > - int pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode); > - if (pixel_multiplier > 1) { > - dpll |= (pixel_multiplier - 1) << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT; > - } > - dpll |= DPLL_DVO_HIGH_SPEED; > - } > - if (is_dp && !is_cpu_edp) > - dpll |= DPLL_DVO_HIGH_SPEED; > - > - /* compute bitmask from p1 value */ > - dpll |= (1 << (clock.p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT; > - /* also FPA1 */ > - dpll |= (1 << (clock.p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT; > - > - switch (clock.p2) { > - case 5: > - dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_5; > - break; > - case 7: > - dpll |= DPLLB_LVDS_P2_CLOCK_DIV_7; > - break; > - case 10: > - dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_10; > - break; > - case 14: > - dpll |= DPLLB_LVDS_P2_CLOCK_DIV_14; > - break; > - } > - > - if (is_sdvo && is_tv) > - dpll |= PLL_REF_INPUT_TVCLKINBC; > - else if (is_tv) > - /* XXX: just matching BIOS for now */ > - /* dpll |= PLL_REF_INPUT_TVCLKINBC; */ > - dpll |= 3; > - else if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) > - dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN; > - else > - dpll |= PLL_REF_INPUT_DREFCLK; > - > DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe); > drm_mode_debug_printmodeline(mode); > > - /* CPU eDP is the only output that doesn't need a PCH PLL of its own on > - * pre-Haswell/LPT generation */ > - if (HAS_PCH_LPT(dev)) { > - DRM_DEBUG_KMS("LPT detected: no PLL for pipe %d necessary\n", > - pipe); > - } else if (!is_cpu_edp) { > - struct intel_pch_pll *pll; > - > - pll = intel_get_pch_pll(intel_crtc, dpll, fp); > - if (pll == NULL) { > - DRM_DEBUG_DRIVER("failed to find PLL for pipe %d\n", > - pipe); > - return -EINVAL; > + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) { Since the plan is to move all the hsw_crtc_stuff out into it's own function I'd prefer a !HAS_PCH_LPT check here. > + /* CPU eDP is the only output that doesn't need a PCH PLL of its > + * own */ > + if (is_cpu_edp) { > + intel_put_pch_pll(intel_crtc); > + } else { > + ok = ironlake_pch_pll_set(crtc, adjusted_mode, &clock, > + has_reduced_clock, > + &reduced_clock); > + if (!ok) { > + DRM_ERROR("Failed to find PLL for pipe %d\n", > + pipe); > + return -EINVAL; > + } > } > - } else > - intel_put_pch_pll(intel_crtc); > + } > > /* The LVDS pin pair needs to be on before the DPLLs are enabled. > * This is an exception to the general rule that mode_set doesn't turn > @@ -5055,31 +5126,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > > ironlake_set_m_n(crtc, mode, adjusted_mode); > > - if (intel_crtc->pch_pll) { > - I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll); > - > - /* Wait for the clocks to stabilize. */ > - POSTING_READ(intel_crtc->pch_pll->pll_reg); > - udelay(150); > - > - /* The pixel multiplier can only be updated once the > - * DPLL is enabled and the clocks are stable. > - * > - * So write it again. > - */ > - I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll); > - } > - > - intel_crtc->lowfreq_avail = false; > - if (intel_crtc->pch_pll) { > - if (is_lvds && has_reduced_clock && i915_powersave) { > - I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp2); > - intel_crtc->lowfreq_avail = true; > - } else { > - I915_WRITE(intel_crtc->pch_pll->fp1_reg, fp); > - } > - } > - Again, you're moving registers around - see the comment about that the lvds pin pairs need to be enabled _before_ we fire up the pll. Yeah, this is a mess and I think we should move the actual pll enabling to the crtc_enable stage (and the also move the lvds pin pair/port enabling in there). So - the actuall pll enabling needs to stay here. - maybe call the function update_pch_pll instead of set_pch_pll. This is more in line with Jesse's similar refactoring of i9xx_crtc_mode_set, where he called the split-out pll functions update_pll, too. Cheers, Daniel > ironlake_set_pipe_timings(intel_crtc, mode, adjusted_mode); > > if (is_cpu_edp) > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch