| Message ID | 1456736924-4259-1-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 29-02-16 om 10:08 schreef Ander Conselvan de Oliveira: > Include DPLL0 in the managed dplls for SKL/KBL. While it has to be kept > enabled because of it driving CDCLK, it is better to special case that > inside the DPLL code than in the higher level. > > v2: Use INTEL_DPLL_ALWAYS_ON flag. (Ander) > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > --- > I wasn't able to test this patch. Would really appreciate a tested by > from someone. Unfortunately no skl with eDP here either. :( > drivers/gpu/drm/i915/intel_ddi.c | 21 ------ > drivers/gpu/drm/i915/intel_dp.c | 52 +------------- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 131 +++++++++++++++++++++++++++------- > drivers/gpu/drm/i915/intel_dpll_mgr.h | 7 +- > 4 files changed, 109 insertions(+), 102 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 3cb9f36..8ef4d2e 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1008,9 +1008,6 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, > { > struct intel_shared_dpll *pll; > > - if (intel_encoder->type == INTEL_OUTPUT_EDP) > - return true; > - > pll = intel_get_shared_dpll(intel_crtc, crtc_state, intel_encoder); > if (pll == NULL) { > DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > @@ -1570,24 +1567,6 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, > uint32_t dpll = pipe_config->ddi_pll_sel; > uint32_t val; > > - /* > - * DPLL0 is used for eDP and is the only "private" DPLL (as > - * opposed to shared) on SKL > - */ > - if (encoder->type == INTEL_OUTPUT_EDP) { > - WARN_ON(dpll != SKL_DPLL0); > - > - val = I915_READ(DPLL_CTRL1); > - > - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | > - DPLL_CTRL1_SSC(dpll) | > - DPLL_CTRL1_LINK_RATE_MASK(dpll)); > - val |= pipe_config->dpll_hw_state.ctrl1 << (dpll * 6); > - > - I915_WRITE(DPLL_CTRL1, val); > - POSTING_READ(DPLL_CTRL1); > - } > - > /* DDI -> PLL mapping */ > val = I915_READ(DPLL_CTRL2); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 5be6892..9fef877 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1229,52 +1229,6 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector) > intel_connector_unregister(intel_connector); > } > > -static void > -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config) > -{ > - u32 ctrl1; > - > - memset(&pipe_config->dpll_hw_state, 0, > - sizeof(pipe_config->dpll_hw_state)); > - > - pipe_config->ddi_pll_sel = SKL_DPLL0; > - pipe_config->dpll_hw_state.cfgcr1 = 0; > - pipe_config->dpll_hw_state.cfgcr2 = 0; > - > - ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0); > - switch (pipe_config->port_clock / 2) { > - case 81000: > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, > - SKL_DPLL0); > - break; > - case 135000: > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, > - SKL_DPLL0); > - break; > - case 270000: > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, > - SKL_DPLL0); > - break; > - case 162000: > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, > - SKL_DPLL0); > - break; > - /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which > - results in CDCLK change. Need to handle the change of CDCLK by > - disabling pipes and re-enabling them */ > - case 108000: > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, > - SKL_DPLL0); > - break; > - case 216000: > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, > - SKL_DPLL0); > - break; > - > - } > - pipe_config->dpll_hw_state.ctrl1 = ctrl1; > -} > - > static int > intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates) > { > @@ -1632,11 +1586,7 @@ found: > &pipe_config->dp_m2_n2); > } > > - if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) && is_edp(intel_dp)) > - skl_edp_set_pll_config(pipe_config); > - else if (IS_BROXTON(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) > - /* handled in ddi */; > - else > + if (!HAS_DDI(dev)) > intel_dp_set_clock(encoder, pipe_config); > > return true; > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index d371680..6ec7836 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -785,7 +785,12 @@ struct skl_dpll_regs { > }; > > /* this array is indexed by the *shared* pll id */ > -static const struct skl_dpll_regs skl_dpll_regs[3] = { > +static const struct skl_dpll_regs skl_dpll_regs[4] = { > + { > + /* DPLL 0 */ > + .ctl = LCPLL1_CTL, > + /* DPLL 0 doesn't support HDMI mode */ > + }, > { > /* DPLL 1 */ > .ctl = LCPLL2_CTL, > @@ -806,24 +811,29 @@ static const struct skl_dpll_regs skl_dpll_regs[3] = { > }, > }; > > -static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, > - struct intel_shared_dpll *pll) > +static void skl_ddi_pll_write_ctrl1(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll) > { > uint32_t val; > - unsigned int dpll; > - const struct skl_dpll_regs *regs = skl_dpll_regs; > - > - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */ > - dpll = pll->id + 1; > > val = I915_READ(DPLL_CTRL1); > > - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | DPLL_CTRL1_SSC(dpll) | > - DPLL_CTRL1_LINK_RATE_MASK(dpll)); > - val |= pll->config.hw_state.ctrl1 << (dpll * 6); > + val &= ~(DPLL_CTRL1_HDMI_MODE(pll->id) | DPLL_CTRL1_SSC(pll->id) | > + DPLL_CTRL1_LINK_RATE_MASK(pll->id)); > + val |= pll->config.hw_state.ctrl1 << (pll->id * 6); > > I915_WRITE(DPLL_CTRL1, val); > POSTING_READ(DPLL_CTRL1); > +} > + > +static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll) > +{ > + const struct skl_dpll_regs *regs = skl_dpll_regs; > + > + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); > + > + skl_ddi_pll_write_ctrl1(dev_priv, pll); > > I915_WRITE(regs[pll->id].cfgcr1, pll->config.hw_state.cfgcr1); > I915_WRITE(regs[pll->id].cfgcr2, pll->config.hw_state.cfgcr2); > @@ -834,8 +844,15 @@ static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, > I915_WRITE(regs[pll->id].ctl, > I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE); > > - if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(dpll), 5)) > - DRM_ERROR("DPLL %d not locked\n", dpll); > + if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(pll->id), 5)) > + DRM_ERROR("DPLL %d not locked\n", pll->id); > +} > + > +static void skl_ddi_dpll0_enable(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll) > +{ > + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); > + skl_ddi_pll_write_ctrl1(dev_priv, pll); > } > > static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv, > @@ -849,32 +866,35 @@ static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv, > POSTING_READ(regs[pll->id].ctl); > } > > +static void skl_ddi_dpll0_disable(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll) > +{ > +} > + > static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll, > struct intel_dpll_hw_state *hw_state) > { > uint32_t val; > - unsigned int dpll; > const struct skl_dpll_regs *regs = skl_dpll_regs; > bool ret; > > + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); > + > if (!intel_display_power_get_if_enabled(dev_priv, POWER_DOMAIN_PLLS)) > return false; > > ret = false; > > - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */ > - dpll = pll->id + 1; > - > val = I915_READ(regs[pll->id].ctl); > if (!(val & LCPLL_PLL_ENABLE)) > goto out; > > val = I915_READ(DPLL_CTRL1); > - hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f; > + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; > > /* avoid reading back stale values if HDMI mode is not enabled */ > - if (val & DPLL_CTRL1_HDMI_MODE(dpll)) { > + if (val & DPLL_CTRL1_HDMI_MODE(pll->id)) { > hw_state->cfgcr1 = I915_READ(regs[pll->id].cfgcr1); > hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2); > } > @@ -886,6 +906,37 @@ out: > return ret; > } > > +static bool skl_ddi_dpll0_get_hw_state(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll, > + struct intel_dpll_hw_state *hw_state) > +{ > + uint32_t val; > + const struct skl_dpll_regs *regs = skl_dpll_regs; > + bool ret; > + > + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); I'm all for paranoia, but how can this ever happen? Seems pll0 has its own function pointers, so you'd have to do something like pll1->funcs->get_hw_state(pll0); I don't think this will ever happen, so all the paranoia from these functions that are called from the function pointer can be removed. > + if (!intel_display_power_get_if_enabled(dev_priv, POWER_DOMAIN_PLLS)) > + return false; > + > + ret = false; > + > + /* DPLL0 is always enabled since it drives CDCLK */ > + val = I915_READ(regs[pll->id].ctl); > + if (WARN_ON(!(val & LCPLL_PLL_ENABLE))) > + goto out; > + > + val = I915_READ(DPLL_CTRL1); > + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; > + > + ret = true; > + > +out: > + intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); > + > + return ret; > +} > + > struct skl_wrpll_context { > uint64_t min_deviation; /* current minimal deviation */ > uint64_t central_freq; /* chosen central freq */ > @@ -1165,7 +1216,8 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > DPLL_CFGCR2_PDIV(wrpll_params.pdiv) | > wrpll_params.central_freq; > } else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || > - encoder->type == INTEL_OUTPUT_DP_MST) { > + encoder->type == INTEL_OUTPUT_DP_MST || > + encoder->type == INTEL_OUTPUT_EDP) { > switch (crtc_state->port_clock / 2) { > case 81000: > ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0); > @@ -1176,6 +1228,19 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, > case 270000: > ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0); > break; > + /* eDP 1.4 rates */ > + case 162000: > + ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, 0); > + break; > + /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which > + results in CDCLK change. Need to handle the change of CDCLK by > + disabling pipes and re-enabling them */ Sounds like we really need to start supporting cdclk changes on SKL, if it's based on this pll? modeset_calc_cdclk and modeset_commit_cdclk. ~Maarten
On Thu, 2016-03-03 at 14:33 +0100, Maarten Lankhorst wrote: > Op 29-02-16 om 10:08 schreef Ander Conselvan de Oliveira: > > Include DPLL0 in the managed dplls for SKL/KBL. While it has to be kept > > enabled because of it driving CDCLK, it is better to special case that > > inside the DPLL code than in the higher level. > > > > v2: Use INTEL_DPLL_ALWAYS_ON flag. (Ander) > > > > Signed-off-by: Ander Conselvan de Oliveira < > > ander.conselvan.de.oliveira@intel.com> > > > > --- > > I wasn't able to test this patch. Would really appreciate a tested by > > from someone. > Unfortunately no skl with eDP here either. :( > > drivers/gpu/drm/i915/intel_ddi.c | 21 ------ > > drivers/gpu/drm/i915/intel_dp.c | 52 +------------- > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 131 +++++++++++++++++++++++++++---- > > --- > > drivers/gpu/drm/i915/intel_dpll_mgr.h | 7 +- > > 4 files changed, 109 insertions(+), 102 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 3cb9f36..8ef4d2e 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1008,9 +1008,6 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, > > { > > struct intel_shared_dpll *pll; > > > > - if (intel_encoder->type == INTEL_OUTPUT_EDP) > > - return true; > > - > > pll = intel_get_shared_dpll(intel_crtc, crtc_state, intel_encoder); > > if (pll == NULL) { > > DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > > @@ -1570,24 +1567,6 @@ void intel_ddi_clk_select(struct intel_encoder > > *encoder, > > uint32_t dpll = pipe_config->ddi_pll_sel; > > uint32_t val; > > > > - /* > > - * DPLL0 is used for eDP and is the only "private" DPLL (as > > - * opposed to shared) on SKL > > - */ > > - if (encoder->type == INTEL_OUTPUT_EDP) { > > - WARN_ON(dpll != SKL_DPLL0); > > - > > - val = I915_READ(DPLL_CTRL1); > > - > > - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | > > - DPLL_CTRL1_SSC(dpll) | > > - DPLL_CTRL1_LINK_RATE_MASK(dpll)); > > - val |= pipe_config->dpll_hw_state.ctrl1 << (dpll * > > 6); > > - > > - I915_WRITE(DPLL_CTRL1, val); > > - POSTING_READ(DPLL_CTRL1); > > - } > > - > > /* DDI -> PLL mapping */ > > val = I915_READ(DPLL_CTRL2); > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 5be6892..9fef877 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1229,52 +1229,6 @@ intel_dp_connector_unregister(struct intel_connector > > *intel_connector) > > intel_connector_unregister(intel_connector); > > } > > > > -static void > > -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config) > > -{ > > - u32 ctrl1; > > - > > - memset(&pipe_config->dpll_hw_state, 0, > > - sizeof(pipe_config->dpll_hw_state)); > > - > > - pipe_config->ddi_pll_sel = SKL_DPLL0; > > - pipe_config->dpll_hw_state.cfgcr1 = 0; > > - pipe_config->dpll_hw_state.cfgcr2 = 0; > > - > > - ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0); > > - switch (pipe_config->port_clock / 2) { > > - case 81000: > > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, > > - SKL_DPLL0); > > - break; > > - case 135000: > > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, > > - SKL_DPLL0); > > - break; > > - case 270000: > > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, > > - SKL_DPLL0); > > - break; > > - case 162000: > > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, > > - SKL_DPLL0); > > - break; > > - /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which > > - results in CDCLK change. Need to handle the change of CDCLK by > > - disabling pipes and re-enabling them */ > > - case 108000: > > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, > > - SKL_DPLL0); > > - break; > > - case 216000: > > - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, > > - SKL_DPLL0); > > - break; > > - > > - } > > - pipe_config->dpll_hw_state.ctrl1 = ctrl1; > > -} > > - > > static int > > intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates) > > { > > @@ -1632,11 +1586,7 @@ found: > > &pipe_config->dp_m2_n2); > > } > > > > - if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) && is_edp(intel_dp)) > > - skl_edp_set_pll_config(pipe_config); > > - else if (IS_BROXTON(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) > > - /* handled in ddi */; > > - else > > + if (!HAS_DDI(dev)) > > intel_dp_set_clock(encoder, pipe_config); > > > > return true; > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > index d371680..6ec7836 100644 > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > @@ -785,7 +785,12 @@ struct skl_dpll_regs { > > }; > > > > /* this array is indexed by the *shared* pll id */ > > -static const struct skl_dpll_regs skl_dpll_regs[3] = { > > +static const struct skl_dpll_regs skl_dpll_regs[4] = { > > + { > > + /* DPLL 0 */ > > + .ctl = LCPLL1_CTL, > > + /* DPLL 0 doesn't support HDMI mode */ > > + }, > > { > > /* DPLL 1 */ > > .ctl = LCPLL2_CTL, > > @@ -806,24 +811,29 @@ static const struct skl_dpll_regs skl_dpll_regs[3] = { > > }, > > }; > > > > -static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, > > - struct intel_shared_dpll *pll) > > +static void skl_ddi_pll_write_ctrl1(struct drm_i915_private *dev_priv, > > + struct intel_shared_dpll *pll) > > { > > uint32_t val; > > - unsigned int dpll; > > - const struct skl_dpll_regs *regs = skl_dpll_regs; > > - > > - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 > > */ > > - dpll = pll->id + 1; > > > > val = I915_READ(DPLL_CTRL1); > > > > - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | DPLL_CTRL1_SSC(dpll) | > > - DPLL_CTRL1_LINK_RATE_MASK(dpll)); > > - val |= pll->config.hw_state.ctrl1 << (dpll * 6); > > + val &= ~(DPLL_CTRL1_HDMI_MODE(pll->id) | DPLL_CTRL1_SSC(pll->id) | > > + DPLL_CTRL1_LINK_RATE_MASK(pll->id)); > > + val |= pll->config.hw_state.ctrl1 << (pll->id * 6); > > > > I915_WRITE(DPLL_CTRL1, val); > > POSTING_READ(DPLL_CTRL1); > > +} > > + > > +static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, > > + struct intel_shared_dpll *pll) > > +{ > > + const struct skl_dpll_regs *regs = skl_dpll_regs; > > + > > + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); > > + > > + skl_ddi_pll_write_ctrl1(dev_priv, pll); > > > > I915_WRITE(regs[pll->id].cfgcr1, pll->config.hw_state.cfgcr1); > > I915_WRITE(regs[pll->id].cfgcr2, pll->config.hw_state.cfgcr2); > > @@ -834,8 +844,15 @@ static void skl_ddi_pll_enable(struct drm_i915_private > > *dev_priv, > > I915_WRITE(regs[pll->id].ctl, > > I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE); > > > > - if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(dpll), 5)) > > - DRM_ERROR("DPLL %d not locked\n", dpll); > > + if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(pll->id), 5)) > > + DRM_ERROR("DPLL %d not locked\n", pll->id); > > +} > > + > > +static void skl_ddi_dpll0_enable(struct drm_i915_private *dev_priv, > > + struct intel_shared_dpll *pll) > > +{ > > + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); > > + skl_ddi_pll_write_ctrl1(dev_priv, pll); > > } > > > > static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv, > > @@ -849,32 +866,35 @@ static void skl_ddi_pll_disable(struct > > drm_i915_private *dev_priv, > > POSTING_READ(regs[pll->id].ctl); > > } > > > > +static void skl_ddi_dpll0_disable(struct drm_i915_private *dev_priv, > > + struct intel_shared_dpll *pll) > > +{ > > +} > > + > > static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, > > struct intel_shared_dpll *pll, > > struct intel_dpll_hw_state *hw_state) > > { > > uint32_t val; > > - unsigned int dpll; > > const struct skl_dpll_regs *regs = skl_dpll_regs; > > bool ret; > > > > + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); > > + > > if (!intel_display_power_get_if_enabled(dev_priv, > > POWER_DOMAIN_PLLS)) > > return false; > > > > ret = false; > > > > - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 > > */ > > - dpll = pll->id + 1; > > - > > val = I915_READ(regs[pll->id].ctl); > > if (!(val & LCPLL_PLL_ENABLE)) > > goto out; > > > > val = I915_READ(DPLL_CTRL1); > > - hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f; > > + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; > > > > /* avoid reading back stale values if HDMI mode is not enabled */ > > - if (val & DPLL_CTRL1_HDMI_MODE(dpll)) { > > + if (val & DPLL_CTRL1_HDMI_MODE(pll->id)) { > > hw_state->cfgcr1 = I915_READ(regs[pll->id].cfgcr1); > > hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2); > > } > > @@ -886,6 +906,37 @@ out: > > return ret; > > } > > > > +static bool skl_ddi_dpll0_get_hw_state(struct drm_i915_private *dev_priv, > > + struct intel_shared_dpll *pll, > > + struct intel_dpll_hw_state > > *hw_state) > > +{ > > + uint32_t val; > > + const struct skl_dpll_regs *regs = skl_dpll_regs; > > + bool ret; > > + > > + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); > I'm all for paranoia, but how can this ever happen? > > Seems pll0 has its own function pointers, so you'd have to do something like > pll1->funcs->get_hw_state(pll0); > > I don't think this will ever happen, so all the paranoia from these functions > that are called from the function pointer can be removed. I went a bit overboard. Will respin. > > + if (!intel_display_power_get_if_enabled(dev_priv, > > POWER_DOMAIN_PLLS)) > > + return false; > > + > > + ret = false; > > + > > + /* DPLL0 is always enabled since it drives CDCLK */ > > + val = I915_READ(regs[pll->id].ctl); > > + if (WARN_ON(!(val & LCPLL_PLL_ENABLE))) > > + goto out; > > + > > + val = I915_READ(DPLL_CTRL1); > > + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; > > + > > + ret = true; > > + > > +out: > > + intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); > > + > > + return ret; > > +} > > + > > struct skl_wrpll_context { > > uint64_t min_deviation; /* current minimal deviation > > */ > > uint64_t central_freq; /* chosen central freq */ > > @@ -1165,7 +1216,8 @@ skl_get_dpll(struct intel_crtc *crtc, struct > > intel_crtc_state *crtc_state, > > DPLL_CFGCR2_PDIV(wrpll_params.pdiv) | > > wrpll_params.central_freq; > > } else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || > > - encoder->type == INTEL_OUTPUT_DP_MST) { > > + encoder->type == INTEL_OUTPUT_DP_MST || > > + encoder->type == INTEL_OUTPUT_EDP) { > > switch (crtc_state->port_clock / 2) { > > case 81000: > > ctrl1 |= > > DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0); > > @@ -1176,6 +1228,19 @@ skl_get_dpll(struct intel_crtc *crtc, struct > > intel_crtc_state *crtc_state, > > case 270000: > > ctrl1 |= > > DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0); > > break; > > + /* eDP 1.4 rates */ > > + case 162000: > > + ctrl1 |= > > DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, 0); > > + break; > > + /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is > > 8640 which > > + results in CDCLK change. Need to handle the change of CDCLK > > by > > + disabling pipes and re-enabling them */ > Sounds like we really need to start supporting cdclk changes on SKL, if it's > based on this pll? > modeset_calc_cdclk and modeset_commit_cdclk. Clint Taylor has been working on it: https://lists.freedesktop.org/archives/intel-gfx/2016-February/087796.html Ander
Op 03-03-16 om 14:40 schreef Ander Conselvan De Oliveira: > On Thu, 2016-03-03 at 14:33 +0100, Maarten Lankhorst wrote: >> Op 29-02-16 om 10:08 schreef Ander Conselvan de Oliveira: >>> Include DPLL0 in the managed dplls for SKL/KBL. While it has to be kept >>> enabled because of it driving CDCLK, it is better to special case that >>> inside the DPLL code than in the higher level. >>> >>> v2: Use INTEL_DPLL_ALWAYS_ON flag. (Ander) >>> >>> Signed-off-by: Ander Conselvan de Oliveira < >>> ander.conselvan.de.oliveira@intel.com> >>> >>> --- >>> I wasn't able to test this patch. Would really appreciate a tested by >>> from someone. >> Unfortunately no skl with eDP here either. :( >>> drivers/gpu/drm/i915/intel_ddi.c | 21 ------ >>> drivers/gpu/drm/i915/intel_dp.c | 52 +------------- >>> drivers/gpu/drm/i915/intel_dpll_mgr.c | 131 +++++++++++++++++++++++++++---- >>> --- >>> drivers/gpu/drm/i915/intel_dpll_mgr.h | 7 +- >>> 4 files changed, 109 insertions(+), 102 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c >>> b/drivers/gpu/drm/i915/intel_ddi.c >>> index 3cb9f36..8ef4d2e 100644 >>> --- a/drivers/gpu/drm/i915/intel_ddi.c >>> +++ b/drivers/gpu/drm/i915/intel_ddi.c >>> @@ -1008,9 +1008,6 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, >>> { >>> struct intel_shared_dpll *pll; >>> >>> - if (intel_encoder->type == INTEL_OUTPUT_EDP) >>> - return true; >>> - >>> pll = intel_get_shared_dpll(intel_crtc, crtc_state, intel_encoder); >>> if (pll == NULL) { >>> DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", >>> @@ -1570,24 +1567,6 @@ void intel_ddi_clk_select(struct intel_encoder >>> *encoder, >>> uint32_t dpll = pipe_config->ddi_pll_sel; >>> uint32_t val; >>> >>> - /* >>> - * DPLL0 is used for eDP and is the only "private" DPLL (as >>> - * opposed to shared) on SKL >>> - */ >>> - if (encoder->type == INTEL_OUTPUT_EDP) { >>> - WARN_ON(dpll != SKL_DPLL0); >>> - >>> - val = I915_READ(DPLL_CTRL1); >>> - >>> - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | >>> - DPLL_CTRL1_SSC(dpll) | >>> - DPLL_CTRL1_LINK_RATE_MASK(dpll)); >>> - val |= pipe_config->dpll_hw_state.ctrl1 << (dpll * >>> 6); >>> - >>> - I915_WRITE(DPLL_CTRL1, val); >>> - POSTING_READ(DPLL_CTRL1); >>> - } >>> - >>> /* DDI -> PLL mapping */ >>> val = I915_READ(DPLL_CTRL2); >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c >>> b/drivers/gpu/drm/i915/intel_dp.c >>> index 5be6892..9fef877 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -1229,52 +1229,6 @@ intel_dp_connector_unregister(struct intel_connector >>> *intel_connector) >>> intel_connector_unregister(intel_connector); >>> } >>> >>> -static void >>> -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config) >>> -{ >>> - u32 ctrl1; >>> - >>> - memset(&pipe_config->dpll_hw_state, 0, >>> - sizeof(pipe_config->dpll_hw_state)); >>> - >>> - pipe_config->ddi_pll_sel = SKL_DPLL0; >>> - pipe_config->dpll_hw_state.cfgcr1 = 0; >>> - pipe_config->dpll_hw_state.cfgcr2 = 0; >>> - >>> - ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0); >>> - switch (pipe_config->port_clock / 2) { >>> - case 81000: >>> - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, >>> - SKL_DPLL0); >>> - break; >>> - case 135000: >>> - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, >>> - SKL_DPLL0); >>> - break; >>> - case 270000: >>> - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, >>> - SKL_DPLL0); >>> - break; >>> - case 162000: >>> - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, >>> - SKL_DPLL0); >>> - break; >>> - /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which >>> - results in CDCLK change. Need to handle the change of CDCLK by >>> - disabling pipes and re-enabling them */ >>> - case 108000: >>> - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, >>> - SKL_DPLL0); >>> - break; >>> - case 216000: >>> - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, >>> - SKL_DPLL0); >>> - break; >>> - >>> - } >>> - pipe_config->dpll_hw_state.ctrl1 = ctrl1; >>> -} >>> - >>> static int >>> intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates) >>> { >>> @@ -1632,11 +1586,7 @@ found: >>> &pipe_config->dp_m2_n2); >>> } >>> >>> - if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) && is_edp(intel_dp)) >>> - skl_edp_set_pll_config(pipe_config); >>> - else if (IS_BROXTON(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) >>> - /* handled in ddi */; >>> - else >>> + if (!HAS_DDI(dev)) >>> intel_dp_set_clock(encoder, pipe_config); >>> >>> return true; >>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c >>> b/drivers/gpu/drm/i915/intel_dpll_mgr.c >>> index d371680..6ec7836 100644 >>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c >>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c >>> @@ -785,7 +785,12 @@ struct skl_dpll_regs { >>> }; >>> >>> /* this array is indexed by the *shared* pll id */ >>> -static const struct skl_dpll_regs skl_dpll_regs[3] = { >>> +static const struct skl_dpll_regs skl_dpll_regs[4] = { >>> + { >>> + /* DPLL 0 */ >>> + .ctl = LCPLL1_CTL, >>> + /* DPLL 0 doesn't support HDMI mode */ >>> + }, >>> { >>> /* DPLL 1 */ >>> .ctl = LCPLL2_CTL, >>> @@ -806,24 +811,29 @@ static const struct skl_dpll_regs skl_dpll_regs[3] = { >>> }, >>> }; >>> >>> -static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, >>> - struct intel_shared_dpll *pll) >>> +static void skl_ddi_pll_write_ctrl1(struct drm_i915_private *dev_priv, >>> + struct intel_shared_dpll *pll) >>> { >>> uint32_t val; >>> - unsigned int dpll; >>> - const struct skl_dpll_regs *regs = skl_dpll_regs; >>> - >>> - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 >>> */ >>> - dpll = pll->id + 1; >>> >>> val = I915_READ(DPLL_CTRL1); >>> >>> - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | DPLL_CTRL1_SSC(dpll) | >>> - DPLL_CTRL1_LINK_RATE_MASK(dpll)); >>> - val |= pll->config.hw_state.ctrl1 << (dpll * 6); >>> + val &= ~(DPLL_CTRL1_HDMI_MODE(pll->id) | DPLL_CTRL1_SSC(pll->id) | >>> + DPLL_CTRL1_LINK_RATE_MASK(pll->id)); >>> + val |= pll->config.hw_state.ctrl1 << (pll->id * 6); >>> >>> I915_WRITE(DPLL_CTRL1, val); >>> POSTING_READ(DPLL_CTRL1); >>> +} >>> + >>> +static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, >>> + struct intel_shared_dpll *pll) >>> +{ >>> + const struct skl_dpll_regs *regs = skl_dpll_regs; >>> + >>> + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); >>> + >>> + skl_ddi_pll_write_ctrl1(dev_priv, pll); >>> >>> I915_WRITE(regs[pll->id].cfgcr1, pll->config.hw_state.cfgcr1); >>> I915_WRITE(regs[pll->id].cfgcr2, pll->config.hw_state.cfgcr2); >>> @@ -834,8 +844,15 @@ static void skl_ddi_pll_enable(struct drm_i915_private >>> *dev_priv, >>> I915_WRITE(regs[pll->id].ctl, >>> I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE); >>> >>> - if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(dpll), 5)) >>> - DRM_ERROR("DPLL %d not locked\n", dpll); >>> + if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(pll->id), 5)) >>> + DRM_ERROR("DPLL %d not locked\n", pll->id); >>> +} >>> + >>> +static void skl_ddi_dpll0_enable(struct drm_i915_private *dev_priv, >>> + struct intel_shared_dpll *pll) >>> +{ >>> + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); >>> + skl_ddi_pll_write_ctrl1(dev_priv, pll); >>> } >>> >>> static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv, >>> @@ -849,32 +866,35 @@ static void skl_ddi_pll_disable(struct >>> drm_i915_private *dev_priv, >>> POSTING_READ(regs[pll->id].ctl); >>> } >>> >>> +static void skl_ddi_dpll0_disable(struct drm_i915_private *dev_priv, >>> + struct intel_shared_dpll *pll) >>> +{ >>> +} >>> + >>> static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, >>> struct intel_shared_dpll *pll, >>> struct intel_dpll_hw_state *hw_state) >>> { >>> uint32_t val; >>> - unsigned int dpll; >>> const struct skl_dpll_regs *regs = skl_dpll_regs; >>> bool ret; >>> >>> + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); >>> + >>> if (!intel_display_power_get_if_enabled(dev_priv, >>> POWER_DOMAIN_PLLS)) >>> return false; >>> >>> ret = false; >>> >>> - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 >>> */ >>> - dpll = pll->id + 1; >>> - >>> val = I915_READ(regs[pll->id].ctl); >>> if (!(val & LCPLL_PLL_ENABLE)) >>> goto out; >>> >>> val = I915_READ(DPLL_CTRL1); >>> - hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f; >>> + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; >>> >>> /* avoid reading back stale values if HDMI mode is not enabled */ >>> - if (val & DPLL_CTRL1_HDMI_MODE(dpll)) { >>> + if (val & DPLL_CTRL1_HDMI_MODE(pll->id)) { >>> hw_state->cfgcr1 = I915_READ(regs[pll->id].cfgcr1); >>> hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2); >>> } >>> @@ -886,6 +906,37 @@ out: >>> return ret; >>> } >>> >>> +static bool skl_ddi_dpll0_get_hw_state(struct drm_i915_private *dev_priv, >>> + struct intel_shared_dpll *pll, >>> + struct intel_dpll_hw_state >>> *hw_state) >>> +{ >>> + uint32_t val; >>> + const struct skl_dpll_regs *regs = skl_dpll_regs; >>> + bool ret; >>> + >>> + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); >> I'm all for paranoia, but how can this ever happen? >> >> Seems pll0 has its own function pointers, so you'd have to do something like >> pll1->funcs->get_hw_state(pll0); >> >> I don't think this will ever happen, so all the paranoia from these functions >> that are called from the function pointer can be removed. > I went a bit overboard. Will respin. > >>> + if (!intel_display_power_get_if_enabled(dev_priv, >>> POWER_DOMAIN_PLLS)) >>> + return false; >>> + >>> + ret = false; >>> + >>> + /* DPLL0 is always enabled since it drives CDCLK */ >>> + val = I915_READ(regs[pll->id].ctl); >>> + if (WARN_ON(!(val & LCPLL_PLL_ENABLE))) >>> + goto out; >>> + >>> + val = I915_READ(DPLL_CTRL1); >>> + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; >>> + >>> + ret = true; >>> + >>> +out: >>> + intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); >>> + >>> + return ret; >>> +} >>> + >>> struct skl_wrpll_context { >>> uint64_t min_deviation; /* current minimal deviation >>> */ >>> uint64_t central_freq; /* chosen central freq */ >>> @@ -1165,7 +1216,8 @@ skl_get_dpll(struct intel_crtc *crtc, struct >>> intel_crtc_state *crtc_state, >>> DPLL_CFGCR2_PDIV(wrpll_params.pdiv) | >>> wrpll_params.central_freq; >>> } else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || >>> - encoder->type == INTEL_OUTPUT_DP_MST) { >>> + encoder->type == INTEL_OUTPUT_DP_MST || >>> + encoder->type == INTEL_OUTPUT_EDP) { >>> switch (crtc_state->port_clock / 2) { >>> case 81000: >>> ctrl1 |= >>> DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0); >>> @@ -1176,6 +1228,19 @@ skl_get_dpll(struct intel_crtc *crtc, struct >>> intel_crtc_state *crtc_state, >>> case 270000: >>> ctrl1 |= >>> DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0); >>> break; >>> + /* eDP 1.4 rates */ >>> + case 162000: >>> + ctrl1 |= >>> DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, 0); >>> + break; >>> + /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is >>> 8640 which >>> + results in CDCLK change. Need to handle the change of CDCLK >>> by >>> + disabling pipes and re-enabling them */ >> Sounds like we really need to start supporting cdclk changes on SKL, if it's >> based on this pll? >> modeset_calc_cdclk and modeset_commit_cdclk. > Clint Taylor has been working on it: > > https://lists.freedesktop.org/archives/intel-gfx/2016-February/087796.html > Ah indeed, that would definitely help with this TBD!
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 3cb9f36..8ef4d2e 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1008,9 +1008,6 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc, { struct intel_shared_dpll *pll; - if (intel_encoder->type == INTEL_OUTPUT_EDP) - return true; - pll = intel_get_shared_dpll(intel_crtc, crtc_state, intel_encoder); if (pll == NULL) { DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", @@ -1570,24 +1567,6 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, uint32_t dpll = pipe_config->ddi_pll_sel; uint32_t val; - /* - * DPLL0 is used for eDP and is the only "private" DPLL (as - * opposed to shared) on SKL - */ - if (encoder->type == INTEL_OUTPUT_EDP) { - WARN_ON(dpll != SKL_DPLL0); - - val = I915_READ(DPLL_CTRL1); - - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | - DPLL_CTRL1_SSC(dpll) | - DPLL_CTRL1_LINK_RATE_MASK(dpll)); - val |= pipe_config->dpll_hw_state.ctrl1 << (dpll * 6); - - I915_WRITE(DPLL_CTRL1, val); - POSTING_READ(DPLL_CTRL1); - } - /* DDI -> PLL mapping */ val = I915_READ(DPLL_CTRL2); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5be6892..9fef877 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1229,52 +1229,6 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector) intel_connector_unregister(intel_connector); } -static void -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config) -{ - u32 ctrl1; - - memset(&pipe_config->dpll_hw_state, 0, - sizeof(pipe_config->dpll_hw_state)); - - pipe_config->ddi_pll_sel = SKL_DPLL0; - pipe_config->dpll_hw_state.cfgcr1 = 0; - pipe_config->dpll_hw_state.cfgcr2 = 0; - - ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0); - switch (pipe_config->port_clock / 2) { - case 81000: - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, - SKL_DPLL0); - break; - case 135000: - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, - SKL_DPLL0); - break; - case 270000: - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, - SKL_DPLL0); - break; - case 162000: - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, - SKL_DPLL0); - break; - /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which - results in CDCLK change. Need to handle the change of CDCLK by - disabling pipes and re-enabling them */ - case 108000: - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, - SKL_DPLL0); - break; - case 216000: - ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, - SKL_DPLL0); - break; - - } - pipe_config->dpll_hw_state.ctrl1 = ctrl1; -} - static int intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates) { @@ -1632,11 +1586,7 @@ found: &pipe_config->dp_m2_n2); } - if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) && is_edp(intel_dp)) - skl_edp_set_pll_config(pipe_config); - else if (IS_BROXTON(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) - /* handled in ddi */; - else + if (!HAS_DDI(dev)) intel_dp_set_clock(encoder, pipe_config); return true; diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index d371680..6ec7836 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -785,7 +785,12 @@ struct skl_dpll_regs { }; /* this array is indexed by the *shared* pll id */ -static const struct skl_dpll_regs skl_dpll_regs[3] = { +static const struct skl_dpll_regs skl_dpll_regs[4] = { + { + /* DPLL 0 */ + .ctl = LCPLL1_CTL, + /* DPLL 0 doesn't support HDMI mode */ + }, { /* DPLL 1 */ .ctl = LCPLL2_CTL, @@ -806,24 +811,29 @@ static const struct skl_dpll_regs skl_dpll_regs[3] = { }, }; -static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, - struct intel_shared_dpll *pll) +static void skl_ddi_pll_write_ctrl1(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll) { uint32_t val; - unsigned int dpll; - const struct skl_dpll_regs *regs = skl_dpll_regs; - - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */ - dpll = pll->id + 1; val = I915_READ(DPLL_CTRL1); - val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | DPLL_CTRL1_SSC(dpll) | - DPLL_CTRL1_LINK_RATE_MASK(dpll)); - val |= pll->config.hw_state.ctrl1 << (dpll * 6); + val &= ~(DPLL_CTRL1_HDMI_MODE(pll->id) | DPLL_CTRL1_SSC(pll->id) | + DPLL_CTRL1_LINK_RATE_MASK(pll->id)); + val |= pll->config.hw_state.ctrl1 << (pll->id * 6); I915_WRITE(DPLL_CTRL1, val); POSTING_READ(DPLL_CTRL1); +} + +static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll) +{ + const struct skl_dpll_regs *regs = skl_dpll_regs; + + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); + + skl_ddi_pll_write_ctrl1(dev_priv, pll); I915_WRITE(regs[pll->id].cfgcr1, pll->config.hw_state.cfgcr1); I915_WRITE(regs[pll->id].cfgcr2, pll->config.hw_state.cfgcr2); @@ -834,8 +844,15 @@ static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv, I915_WRITE(regs[pll->id].ctl, I915_READ(regs[pll->id].ctl) | LCPLL_PLL_ENABLE); - if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(dpll), 5)) - DRM_ERROR("DPLL %d not locked\n", dpll); + if (wait_for(I915_READ(DPLL_STATUS) & DPLL_LOCK(pll->id), 5)) + DRM_ERROR("DPLL %d not locked\n", pll->id); +} + +static void skl_ddi_dpll0_enable(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll) +{ + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); + skl_ddi_pll_write_ctrl1(dev_priv, pll); } static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv, @@ -849,32 +866,35 @@ static void skl_ddi_pll_disable(struct drm_i915_private *dev_priv, POSTING_READ(regs[pll->id].ctl); } +static void skl_ddi_dpll0_disable(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll) +{ +} + static bool skl_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll, struct intel_dpll_hw_state *hw_state) { uint32_t val; - unsigned int dpll; const struct skl_dpll_regs *regs = skl_dpll_regs; bool ret; + WARN_ON(pll->id == DPLL_ID_SKL_DPLL0); + if (!intel_display_power_get_if_enabled(dev_priv, POWER_DOMAIN_PLLS)) return false; ret = false; - /* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */ - dpll = pll->id + 1; - val = I915_READ(regs[pll->id].ctl); if (!(val & LCPLL_PLL_ENABLE)) goto out; val = I915_READ(DPLL_CTRL1); - hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f; + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; /* avoid reading back stale values if HDMI mode is not enabled */ - if (val & DPLL_CTRL1_HDMI_MODE(dpll)) { + if (val & DPLL_CTRL1_HDMI_MODE(pll->id)) { hw_state->cfgcr1 = I915_READ(regs[pll->id].cfgcr1); hw_state->cfgcr2 = I915_READ(regs[pll->id].cfgcr2); } @@ -886,6 +906,37 @@ out: return ret; } +static bool skl_ddi_dpll0_get_hw_state(struct drm_i915_private *dev_priv, + struct intel_shared_dpll *pll, + struct intel_dpll_hw_state *hw_state) +{ + uint32_t val; + const struct skl_dpll_regs *regs = skl_dpll_regs; + bool ret; + + WARN_ON(pll->id != DPLL_ID_SKL_DPLL0); + + if (!intel_display_power_get_if_enabled(dev_priv, POWER_DOMAIN_PLLS)) + return false; + + ret = false; + + /* DPLL0 is always enabled since it drives CDCLK */ + val = I915_READ(regs[pll->id].ctl); + if (WARN_ON(!(val & LCPLL_PLL_ENABLE))) + goto out; + + val = I915_READ(DPLL_CTRL1); + hw_state->ctrl1 = (val >> (pll->id * 6)) & 0x3f; + + ret = true; + +out: + intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); + + return ret; +} + struct skl_wrpll_context { uint64_t min_deviation; /* current minimal deviation */ uint64_t central_freq; /* chosen central freq */ @@ -1165,7 +1216,8 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, DPLL_CFGCR2_PDIV(wrpll_params.pdiv) | wrpll_params.central_freq; } else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT || - encoder->type == INTEL_OUTPUT_DP_MST) { + encoder->type == INTEL_OUTPUT_DP_MST || + encoder->type == INTEL_OUTPUT_EDP) { switch (crtc_state->port_clock / 2) { case 81000: ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0); @@ -1176,6 +1228,19 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, case 270000: ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0); break; + /* eDP 1.4 rates */ + case 162000: + ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, 0); + break; + /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which + results in CDCLK change. Need to handle the change of CDCLK by + disabling pipes and re-enabling them */ + case 108000: + ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, 0); + break; + case 216000: + ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, 0); + break; } cfgcr1 = cfgcr2 = 0; @@ -1190,13 +1255,18 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, crtc_state->dpll_hw_state.cfgcr1 = cfgcr1; crtc_state->dpll_hw_state.cfgcr2 = cfgcr2; - pll = intel_find_shared_dpll(crtc, crtc_state, - DPLL_ID_SKL_DPLL1, DPLL_ID_SKL_DPLL3); + if (encoder->type == INTEL_OUTPUT_EDP) + pll = intel_find_shared_dpll(crtc, crtc_state, + DPLL_ID_SKL_DPLL0, + DPLL_ID_SKL_DPLL0); + else + pll = intel_find_shared_dpll(crtc, crtc_state, + DPLL_ID_SKL_DPLL1, + DPLL_ID_SKL_DPLL3); if (!pll) return NULL; - /* shared DPLL id 0 is DPLL 1 */ - crtc_state->ddi_pll_sel = pll->id + 1; + crtc_state->ddi_pll_sel = pll->id; intel_reference_shared_dpll(pll, crtc_state); @@ -1209,6 +1279,12 @@ static const struct intel_shared_dpll_funcs skl_ddi_pll_funcs = { .get_hw_state = skl_ddi_pll_get_hw_state, }; +static const struct intel_shared_dpll_funcs skl_ddi_dpll0_funcs = { + .enable = skl_ddi_dpll0_enable, + .disable = skl_ddi_dpll0_disable, + .get_hw_state = skl_ddi_dpll0_get_hw_state, +}; + static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll) { @@ -1624,9 +1700,10 @@ static const struct intel_dpll_mgr hsw_pll_mgr = { }; static const struct dpll_info skl_plls[] = { - { "DPPL 1", DPLL_ID_SKL_DPLL1, &skl_ddi_pll_funcs, 0 }, - { "DPPL 2", DPLL_ID_SKL_DPLL2, &skl_ddi_pll_funcs, 0 }, - { "DPPL 3", DPLL_ID_SKL_DPLL3, &skl_ddi_pll_funcs, 0 }, + { "DPLL 0", DPLL_ID_SKL_DPLL0, &skl_ddi_dpll0_funcs, INTEL_DPLL_ALWAYS_ON }, + { "DPPL 1", DPLL_ID_SKL_DPLL1, &skl_ddi_pll_funcs, 0 }, + { "DPPL 2", DPLL_ID_SKL_DPLL2, &skl_ddi_pll_funcs, 0 }, + { "DPPL 3", DPLL_ID_SKL_DPLL3, &skl_ddi_pll_funcs, 0 }, { NULL, -1, NULL, }, }; diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index adf4706..1d34147 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -54,9 +54,10 @@ enum intel_dpll_id { DPLL_ID_LCPLL_2700 = 5, /* skl */ - DPLL_ID_SKL_DPLL1 = 0, - DPLL_ID_SKL_DPLL2 = 1, - DPLL_ID_SKL_DPLL3 = 2, + DPLL_ID_SKL_DPLL0 = 0, + DPLL_ID_SKL_DPLL1 = 1, + DPLL_ID_SKL_DPLL2 = 2, + DPLL_ID_SKL_DPLL3 = 3, }; #define I915_NUM_PLLS 6
Include DPLL0 in the managed dplls for SKL/KBL. While it has to be kept enabled because of it driving CDCLK, it is better to special case that inside the DPLL code than in the higher level. v2: Use INTEL_DPLL_ALWAYS_ON flag. (Ander) Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- I wasn't able to test this patch. Would really appreciate a tested by from someone. --- drivers/gpu/drm/i915/intel_ddi.c | 21 ------ drivers/gpu/drm/i915/intel_dp.c | 52 +------------- drivers/gpu/drm/i915/intel_dpll_mgr.c | 131 +++++++++++++++++++++++++++------- drivers/gpu/drm/i915/intel_dpll_mgr.h | 7 +- 4 files changed, 109 insertions(+), 102 deletions(-)