On Fri, Sep 27, 2013 at 04:02:29PM -0700, Jesse Barnes wrote: > The global integrated clock source bit resides in DPLL B on VLV, but we > were treating it as a per-pipe resource. It needs to be set whenever > any PLL is active, so pull setting the bit out of vlv_update_pll and > into vlv_enable_pll. Also add a vlv_disable_pll to prevent disabling it > when pipe B shuts down. > > I'm guessing on the references here, I expect this to bite any config > where multiple displays are active or displays are moved from pipe to > pipe. > > v2: re-add bits in vlv_update_pll to keep from confusing the state checker > v3: use enum pipe checks (Daniel) > set CRI clock source early (Ville) > consistently set CRI clock source everywhere (Ville) Btw do we care about the additional power consumption of having that clock running all the time? My long-term plan/idea for these display refclocks was to enable/disable them in the ->modeset_global_resources callback so that we only ever enable what we actually need. Haswell has this somewhat implemented already implicitly through the pc8+ power refcounting. Just a "have you thought about this" comment. > References: https://bugs.freedesktop.org/show_bug.cgi?id=67245 > References: https://bugs.freedesktop.org/show_bug.cgi?id=69693 > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > > int clock Dropped some paste garbage by accident? > --- > drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++++--- > 1 file changed, 33 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 9a83236..1c76a26 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1387,6 +1387,13 @@ static void vlv_enable_pll(struct intel_crtc *crtc) > if (IS_MOBILE(dev_priv->dev) && !IS_I830(dev_priv->dev)) > assert_panel_unlocked(dev_priv, crtc->pipe); > > + /* Make sure the integrated clock source is enabled */ > + if (crtc->pipe == PIPE_B) > + dpll |= DPLL_INTEGRATED_CRI_CLK_VLV; > + else > + I915_WRITE(DPLL(1), I915_READ(DPLL(1)) | s/1/PIPE_B/g > + DPLL_INTEGRATED_CRI_CLK_VLV); > + > I915_WRITE(reg, dpll); > POSTING_READ(reg); > udelay(150); > @@ -1477,6 +1484,20 @@ static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) > POSTING_READ(DPLL(pipe)); > } > > +static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) > +{ > + u32 val = 0; > + > + /* Make sure the pipe isn't still relying on us */ > + assert_pipe_disabled(dev_priv, pipe); > + > + /* Leave integrated clock source enabled */ > + if (pipe == PIPE_B) > + val = DPLL_INTEGRATED_CRI_CLK_VLV; > + I915_WRITE(DPLL(pipe), val); > + POSTING_READ(DPLL(pipe)); > +} > + > void vlv_wait_port_ready(struct drm_i915_private *dev_priv, int port) > { > u32 port_mask; > @@ -3887,7 +3908,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) > if (encoder->post_disable) > encoder->post_disable(encoder); > > - if (!intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI)) > + if (IS_VALLEYVIEW(dev) && !intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI)) > + vlv_disable_pll(dev_priv, pipe); > + else if (!IS_VALLEYVIEW(dev)) > i9xx_disable_pll(dev_priv, pipe); > > intel_crtc->active = false; > @@ -4627,9 +4650,9 @@ static void vlv_update_pll(struct intel_crtc *crtc) > /* Enable DPIO clock input */ > dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV | > DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV; > - if (pipe) > + /* We should never disable this, set it here for state tracking */ > + if (pipe == PIPE_B) > dpll |= DPLL_INTEGRATED_CRI_CLK_VLV; > - > dpll |= DPLL_VCO_ENABLE; > crtc->config.dpll_hw_state.dpll = dpll; > > @@ -10296,8 +10319,15 @@ void i915_disable_vga_mem(struct drm_device *dev) > > void intel_modeset_init_hw(struct drm_device *dev) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > + > intel_prepare_ddi(dev); > > + /* Enable the CRI clock source so we can get at the display */ > + if (IS_VALLEYVIEW(dev)) > + I915_WRITE(DPLL(1), I915_READ(DPLL(1)) | s/1/PIPE_B/g Also there's this --in-reply-to switch for git send-email to keep discussions on the m-l neatly tied up ;-) > + DPLL_INTEGRATED_CRI_CLK_VLV); > + > intel_init_dpio(dev); > > intel_init_clock_gating(dev); > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
