On Thu, Nov 26, 2015 at 01:42:10PM +0200, Ville Syrjälä wrote: > On Thu, Nov 26, 2015 at 09:41:36AM +0100, Daniel Vetter wrote: > > On Wed, Nov 25, 2015 at 04:35:31PM +0200, [email protected] > > wrote: > > > From: Ville Syrjälä <[email protected]> > > > > > > DPLL_SDVO_HIGH_SPEED must be set for SDVO/HDMI/DP, but nowhere is it > > > forbidden to set it for LVDS/CRT as well. So let's move it out of the > > > ironlake_compute_dpll() and just do it on demand in the pll enable hook. > > > This allows the PLL to be shared in more cases when dealing with > > > different output types. > > > > Nowhere is it forbidden feels a bit weak ... what's the benefits? Do we > > really care? > > We get to share the PLL eg. between HDMI and VGA. We already do that > anyway if we clone HDMI+VGA. Othwerwise we always run out of PLLs when > trying to do three pipes with something like VGA+HDMI+DP.
I read through the spec a bit more, and while they don't say things entirely clearly, I think this bit just enables the x5 clock output from the DPLL into the port. CRT doesn't need to deal with individual bits, and so doesn't need this clock. And maybe gating it off at the DPLL saves a bit of power. Based on that I'm thinking that in theory we should be able to shared the DPLL even between CRT and LVDS since the LVDS x7 clock is simply not relevant for CRT. However the spec clearly groups the DAC with SDVO/HDMI/DP when talking about the DPLL. Not entirely sure why that is. If I had a machine with LVDS+CRT I would definitely test out this theory. Anyways, I still think we want this patch. > > > -Daniel > > > > > > > > Note that we must now call the pll enable hook regarless of the current > > > pll state, so that DPLL_SDVO_HIGH_SPEED gets updated appropriately. > > > > > > FIXME: maybe better to add a separate hook for the "pll already enabled" > > > case? > > > > > > Signed-off-by: Ville Syrjälä <[email protected]> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++-------- > > > 1 file changed, 21 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 26cafeea2845..ae58f1105458 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -1908,6 +1908,8 @@ static void intel_enable_shared_dpll(struct > > > intel_crtc *crtc) > > > if (pll->active++) { > > > WARN_ON(!pll->on); > > > assert_shared_dpll_enabled(dev_priv, pll); > > > + /* to update high speed IO clock state */ > > > + pll->enable(dev_priv, pll); > > > return; > > > } > > > WARN_ON(pll->on); > > > @@ -8942,11 +8944,6 @@ static uint32_t ironlake_compute_dpll(struct > > > intel_crtc *intel_crtc, > > > dpll |= (crtc_state->pixel_multiplier - 1) > > > << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT; > > > > > > - if (is_sdvo) > > > - dpll |= DPLL_SDVO_HIGH_SPEED; > > > - if (crtc_state->has_dp_encoder) > > > - dpll |= DPLL_SDVO_HIGH_SPEED; > > > - > > > /* compute bitmask from p1 value */ > > > dpll |= (1 << (crtc_state->dpll.p1 - 1)) << > > > DPLL_FPA01_P1_POST_DIV_SHIFT; > > > /* also FPA1 */ > > > @@ -13626,7 +13623,7 @@ static bool ibx_pch_dpll_get_hw_state(struct > > > drm_i915_private *dev_priv, > > > return false; > > > > > > val = I915_READ(PCH_DPLL(pll->id)); > > > - hw_state->dpll = val; > > > + hw_state->dpll = val & ~DPLL_SDVO_HIGH_SPEED; > > > hw_state->fp0 = I915_READ(PCH_FP0(pll->id)); > > > hw_state->fp1 = I915_READ(PCH_FP1(pll->id)); > > > > > > @@ -13643,10 +13640,26 @@ static void ibx_pch_dpll_mode_set(struct > > > drm_i915_private *dev_priv, > > > static void ibx_pch_dpll_enable(struct drm_i915_private *dev_priv, > > > struct intel_shared_dpll *pll) > > > { > > > + struct drm_device *dev = dev_priv->dev; > > > + struct intel_crtc *crtc; > > > + u32 dpll = pll->config.hw_state.dpll; > > > + > > > + /* Configure high speed IO clock as needed */ > > > + for_each_intel_crtc(dev, crtc) { > > > + if (intel_crtc_to_shared_dpll(crtc) == pll && > > > + crtc->config->base.active && > > > + (crtc->config->has_dp_encoder || > > > + intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) || > > > + intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))) { > > > + dpll |= DPLL_SDVO_HIGH_SPEED; > > > + break; > > > + } > > > + } > > > + > > > /* PCH refclock must be enabled first */ > > > ibx_assert_pch_refclk_enabled(dev_priv); > > > > > > - I915_WRITE(PCH_DPLL(pll->id), pll->config.hw_state.dpll); > > > + I915_WRITE(PCH_DPLL(pll->id), dpll); > > > > > > /* Wait for the clocks to stabilize. */ > > > POSTING_READ(PCH_DPLL(pll->id)); > > > @@ -13657,7 +13670,7 @@ static void ibx_pch_dpll_enable(struct > > > drm_i915_private *dev_priv, > > > * > > > * So write it again. > > > */ > > > - I915_WRITE(PCH_DPLL(pll->id), pll->config.hw_state.dpll); > > > + I915_WRITE(PCH_DPLL(pll->id), dpll); > > > POSTING_READ(PCH_DPLL(pll->id)); > > > udelay(200); > > > } > > > -- > > > 2.4.10 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Ville Syrjälä > Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
