On Fri, Apr 15, 2016 at 04:09:31PM +0300, Jani Nikula wrote: > On Tue, 12 Apr 2016, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Set up DPLL and DPLL_MD even when driving DSI output on VLV/CHV. While > > the DPLL isn't used to provide the clock we still need the refclock, and > > it appears that the pixel repeat factor also has an effect on DSI > > output. So set up eveyrhing in DPLL and DPLL_MD as we would do for > > DP/HDMI/VGA, but don't actually enable the DPLL or configure the > > dividers via DPIO. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > I quickly glanced over 2-5 too, my earlier r-b on them still stand. Thanks. Series pushed to dinq. > > > --- > > drivers/gpu/drm/i915/intel_display.c | 120 +++++++++++++++++++++-------------- > > drivers/gpu/drm/i915/intel_dsi.c | 28 ++------ > > 2 files changed, 80 insertions(+), 68 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 607dc41bcc68..7c74a930f45d 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1530,45 +1530,47 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > > } > > > > +static void _vlv_enable_pll(struct intel_crtc *crtc, > > + const struct intel_crtc_state *pipe_config) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + enum pipe pipe = crtc->pipe; > > + > > + I915_WRITE(DPLL(pipe), pipe_config->dpll_hw_state.dpll); > > + POSTING_READ(DPLL(pipe)); > > + udelay(150); > > + > > + if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) > > + DRM_ERROR("DPLL %d failed to lock\n", pipe); > > +} > > + > > static void vlv_enable_pll(struct intel_crtc *crtc, > > const struct intel_crtc_state *pipe_config) > > { > > - struct drm_device *dev = crtc->base.dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > enum pipe pipe = crtc->pipe; > > - i915_reg_t reg = DPLL(pipe); > > - u32 dpll = pipe_config->dpll_hw_state.dpll; > > > > assert_pipe_disabled(dev_priv, pipe); > > > > /* PLL is protected by panel, make sure we can write it */ > > assert_panel_unlocked(dev_priv, pipe); > > > > - I915_WRITE(reg, dpll); > > - POSTING_READ(reg); > > - udelay(150); > > - > > - if (wait_for(((I915_READ(reg) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) > > - DRM_ERROR("DPLL %d failed to lock\n", pipe); > > + if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) > > + _vlv_enable_pll(crtc, pipe_config); > > > > I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md); > > POSTING_READ(DPLL_MD(pipe)); > > } > > > > -static void chv_enable_pll(struct intel_crtc *crtc, > > - const struct intel_crtc_state *pipe_config) > > + > > +static void _chv_enable_pll(struct intel_crtc *crtc, > > + const struct intel_crtc_state *pipe_config) > > { > > - struct drm_device *dev = crtc->base.dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > enum pipe pipe = crtc->pipe; > > enum dpio_channel port = vlv_pipe_to_channel(pipe); > > u32 tmp; > > > > - assert_pipe_disabled(dev_priv, pipe); > > - > > - /* PLL is protected by panel, make sure we can write it */ > > - assert_panel_unlocked(dev_priv, pipe); > > - > > mutex_lock(&dev_priv->sb_lock); > > > > /* Enable back the 10bit clock to display controller */ > > @@ -1589,6 +1591,21 @@ static void chv_enable_pll(struct intel_crtc *crtc, > > /* Check PLL is locked */ > > if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) > > DRM_ERROR("PLL %d failed to lock\n", pipe); > > +} > > + > > +static void chv_enable_pll(struct intel_crtc *crtc, > > + const struct intel_crtc_state *pipe_config) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + enum pipe pipe = crtc->pipe; > > + > > + assert_pipe_disabled(dev_priv, pipe); > > + > > + /* PLL is protected by panel, make sure we can write it */ > > + assert_panel_unlocked(dev_priv, pipe); > > + > > + if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) > > + _chv_enable_pll(crtc, pipe_config); > > > > if (pipe != PIPE_A) { > > /* > > @@ -6073,14 +6090,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > > if (encoder->pre_pll_enable) > > encoder->pre_pll_enable(encoder); > > > > - if (!intel_crtc->config->has_dsi_encoder) { > > - if (IS_CHERRYVIEW(dev)) { > > - chv_prepare_pll(intel_crtc, intel_crtc->config); > > - chv_enable_pll(intel_crtc, intel_crtc->config); > > - } else { > > - vlv_prepare_pll(intel_crtc, intel_crtc->config); > > - vlv_enable_pll(intel_crtc, intel_crtc->config); > > - } > > + if (IS_CHERRYVIEW(dev)) { > > + chv_prepare_pll(intel_crtc, intel_crtc->config); > > + chv_enable_pll(intel_crtc, intel_crtc->config); > > + } else { > > + vlv_prepare_pll(intel_crtc, intel_crtc->config); > > + vlv_enable_pll(intel_crtc, intel_crtc->config); > > } > > > > for_each_encoder_on_crtc(dev, crtc, encoder) > > @@ -6118,7 +6133,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > struct intel_encoder *encoder; > > struct intel_crtc_state *pipe_config = > > to_intel_crtc_state(crtc->state); > > - int pipe = intel_crtc->pipe; > > + enum pipe pipe = intel_crtc->pipe; > > > > if (WARN_ON(intel_crtc->active)) > > return; > > @@ -7174,11 +7189,15 @@ static void vlv_compute_dpll(struct intel_crtc *crtc, > > struct intel_crtc_state *pipe_config) > > { > > pipe_config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV | > > - DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS | > > - DPLL_VCO_ENABLE | DPLL_EXT_BUFFER_ENABLE_VLV; > > + DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > if (crtc->pipe != PIPE_A) > > pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV; > > > > + /* DPLL not used with DSI, but still need the rest set up */ > > + if (!intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_DSI)) > > + pipe_config->dpll_hw_state.dpll |= DPLL_VCO_ENABLE | > > + DPLL_EXT_BUFFER_ENABLE_VLV; > > + > > pipe_config->dpll_hw_state.dpll_md = > > (pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT; > > } > > @@ -7187,11 +7206,14 @@ static void chv_compute_dpll(struct intel_crtc *crtc, > > struct intel_crtc_state *pipe_config) > > { > > pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV | > > - DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS | > > - DPLL_VCO_ENABLE; > > + DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > if (crtc->pipe != PIPE_A) > > pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV; > > > > + /* DPLL not used with DSI, but still need the rest set up */ > > + if (!intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_DSI)) > > + pipe_config->dpll_hw_state.dpll |= DPLL_VCO_ENABLE; > > + > > pipe_config->dpll_hw_state.dpll_md = > > (pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT; > > } > > @@ -7201,11 +7223,20 @@ static void vlv_prepare_pll(struct intel_crtc *crtc, > > { > > struct drm_device *dev = crtc->base.dev; > > struct drm_i915_private *dev_priv = dev->dev_private; > > - int pipe = crtc->pipe; > > + enum pipe pipe = crtc->pipe; > > u32 mdiv; > > u32 bestn, bestm1, bestm2, bestp1, bestp2; > > u32 coreclk, reg_val; > > > > + /* Enable Refclk */ > > + I915_WRITE(DPLL(pipe), > > + pipe_config->dpll_hw_state.dpll & > > + ~(DPLL_VCO_ENABLE | DPLL_EXT_BUFFER_ENABLE_VLV)); > > + > > + /* No need to actually set up the DPLL with DSI */ > > + if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0) > > + return; > > + > > mutex_lock(&dev_priv->sb_lock); > > > > bestn = pipe_config->dpll.n; > > @@ -7292,14 +7323,21 @@ static void chv_prepare_pll(struct intel_crtc *crtc, > > { > > struct drm_device *dev = crtc->base.dev; > > struct drm_i915_private *dev_priv = dev->dev_private; > > - int pipe = crtc->pipe; > > - i915_reg_t dpll_reg = DPLL(crtc->pipe); > > + enum pipe pipe = crtc->pipe; > > enum dpio_channel port = vlv_pipe_to_channel(pipe); > > u32 loopfilter, tribuf_calcntr; > > u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac; > > u32 dpio_val; > > int vco; > > > > + /* Enable Refclk and SSC */ > > + I915_WRITE(DPLL(pipe), > > + pipe_config->dpll_hw_state.dpll & ~DPLL_VCO_ENABLE); > > + > > + /* No need to actually set up the DPLL with DSI */ > > + if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0) > > + return; > > + > > bestn = pipe_config->dpll.n; > > bestm2_frac = pipe_config->dpll.m2 & 0x3fffff; > > bestm1 = pipe_config->dpll.m1; > > @@ -7310,12 +7348,6 @@ static void chv_prepare_pll(struct intel_crtc *crtc, > > dpio_val = 0; > > loopfilter = 0; > > > > - /* > > - * Enable Refclk and SSC > > - */ > > - I915_WRITE(dpll_reg, > > - pipe_config->dpll_hw_state.dpll & ~DPLL_VCO_ENABLE); > > - > > mutex_lock(&dev_priv->sb_lock); > > > > /* p1 and p2 divider */ > > @@ -7930,9 +7962,6 @@ static int chv_crtc_compute_clock(struct intel_crtc *crtc, > > memset(&crtc_state->dpll_hw_state, 0, > > sizeof(crtc_state->dpll_hw_state)); > > > > - if (crtc_state->has_dsi_encoder) > > - return 0; > > - > > if (!crtc_state->clock_set && > > !chv_find_best_dpll(limit, crtc_state, crtc_state->port_clock, > > refclk, NULL, &crtc_state->dpll)) { > > @@ -7954,9 +7983,6 @@ static int vlv_crtc_compute_clock(struct intel_crtc *crtc, > > memset(&crtc_state->dpll_hw_state, 0, > > sizeof(crtc_state->dpll_hw_state)); > > > > - if (crtc_state->has_dsi_encoder) > > - return 0; > > - > > if (!crtc_state->clock_set && > > !vlv_find_best_dpll(limit, crtc_state, crtc_state->port_clock, > > refclk, NULL, &crtc_state->dpll)) { > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > > index 9ff6435e7d38..22bd42a8aab0 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -311,6 +311,12 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, > > pipe_config->cpu_transcoder = TRANSCODER_DSI_A; > > } > > > > + /* > > + * FIXME move the DSI PLL calc from vlv_enable_dsi_pll() > > + * to .compute_config(). > > + */ > > + pipe_config->clock_set = true; > > + > > return true; > > } > > > > @@ -498,8 +504,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) > > struct drm_device *dev = encoder->base.dev; > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > > - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); > > - enum pipe pipe = intel_crtc->pipe; > > enum port port; > > u32 tmp; > > > > @@ -521,19 +525,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) > > msleep(intel_dsi->panel_on_delay); > > > > if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > > - /* > > - * Disable DPOunit clock gating, can stall pipe > > - * and we need DPLL REFA always enabled > > - */ > > - tmp = I915_READ(DPLL(pipe)); > > - tmp |= DPLL_REF_CLK_ENABLE_VLV; > > - I915_WRITE(DPLL(pipe), tmp); > > - > > - /* update the hw state for DPLL */ > > - intel_crtc->config->dpll_hw_state.dpll = > > - DPLL_INTEGRATED_REF_CLK_VLV | > > - DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS; > > - > > + /* Disable DPOunit clock gating, can stall pipe */ > > tmp = I915_READ(DSPCLK_GATE_D); > > tmp |= DPOUNIT_CLOCK_GATE_DISABLE; > > I915_WRITE(DSPCLK_GATE_D, tmp); > > @@ -832,12 +824,6 @@ static void intel_dsi_get_config(struct intel_encoder *encoder, > > if (IS_BROXTON(dev)) > > bxt_dsi_get_pipe_config(encoder, pipe_config); > > > > - /* > > - * DPLL_MD is not used in case of DSI, reading will get some default value > > - * set dpll_md = 0 > > - */ > > - pipe_config->dpll_hw_state.dpll_md = 0; > > - > > pclk = intel_dsi_get_pclk(encoder, pipe_config->pipe_bpp); > > if (!pclk) > > return; > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx