| Message ID | 1430752821-12428-1-git-send-email-vandana.kannan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6314
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 316/316 316/316
IVB 342/342 342/342
BYT 286/286 286/286
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
Note: You need to pay more attention to line start with '*'
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> On 5/4/2015 8:50 PM, Vandana Kannan wrote: > BUN 1: prop_coeff, int_coeff, tdctargetcnt programming updated and tied to > VCO frequencies. Program i_lockthresh in PORT_PLL_9. > > VCO calculated based on the formula: > Desired Output = Port bit rate in MHz (DisplayPort HBR2 is 5400 MHz) > Fast Clock = Desired Output / 2 > VCO = Fast Clock * P1 * P2 > > Prop_coeff, int_coeff, and tdctargetcnt modified according to above > calculation. > > BUN 2: Port PLLs require additional programming at certain frequencies - > DCO amplitude in PORT_PLL_10 > > Review comments from Siva which were addressed in the initial version of the > patch. > - Change PORT_PLL_LOCK_THRESHOLD to PORT_PLL_LOCK_THRESHOLD_MASK > - Calculate for HDMI > - Correct values for vco = 5.4 > - return in case of invalid vco range > > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 6 ++++ > drivers/gpu/drm/i915/intel_ddi.c | 72 +++++++++++++++++++++++++++++++--------- > 3 files changed, 63 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f3c77ca..cea54ee 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -309,7 +309,7 @@ struct intel_dpll_hw_state { > uint32_t cfgcr1, cfgcr2; > > /* bxt */ > - uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pcsdw12; > + uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pll10, pcsdw12; > }; > > struct intel_shared_dpll_config { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 36805b6..86fdc3e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1185,6 +1185,12 @@ enum skl_disp_power_wells { > #define PORT_PLL_GAIN_CTL(x) ((x) << 16) > /* PORT_PLL_8_A */ > #define PORT_PLL_TARGET_CNT_MASK 0x3FF > +/* PORT_PLL_9_A */ > +#define PORT_PLL_LOCK_THRESHOLD_MASK 0xe > +/* PORT_PLL_10_A */ > +#define PORT_PLL_DCO_AMP_OVR_EN (1<<27) > +#define PORT_PLL_DCO_AMP_MASK 0x3c00 > +#define PORT_PLL_DCO_AMP(x) (x<<10) > #define _PORT_PLL_BASE(port) _PORT3(port, _PORT_PLL_0_A, \ > _PORT_PLL_0_B, \ > _PORT_PLL_0_C) > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 455d44b..3014c37 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1319,18 +1319,20 @@ struct bxt_clk_div { > uint32_t int_coef; > uint32_t gain_ctl; > uint32_t targ_cnt; > + uint32_t dcoampovr_en; > + uint32_t dco_amp; > uint32_t lanestagger; > }; > > /* pre-calculated values for DP linkrates */ > static struct bxt_clk_div bxt_dp_clk_val[7] = { > - /* 162 */ {4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, > - /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, > - /* 216 */ {3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 324 */ {4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 432 */ {3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} > + /* 162 */ {4, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0xd}, > + /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0x18}, > + /* 216 */ {3, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 3, 9, 1, 15, 0xd}, > + /* 324 */ {4, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 432 */ {3, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0x18} > }; > > static bool > @@ -1364,11 +1366,34 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > clk_div.m2_frac = best_clock.m2 & ((1 << 22) - 1); > clk_div.m2_frac_en = clk_div.m2_frac != 0; > > - /* FIXME: set coef, gain, targcnt based on freq band */ > - clk_div.prop_coef = 5; > - clk_div.int_coef = 11; > - clk_div.gain_ctl = 2; > - clk_div.targ_cnt = 9; > + clk_div.dco_amp = 15; > + clk_div.dcoampovr_en = 0; > + if (best_clock.vco >= 6200000 && best_clock.vco <= 6480000) { > + clk_div.prop_coef = 4; > + clk_div.int_coef = 9; > + clk_div.gain_ctl = 3; > + clk_div.targ_cnt = 8; > + } else if ((best_clock.vco > 5400000 && > + best_clock.vco < 6200000) || > + (best_clock.vco >= 4800000 && > + best_clock.vco < 5400000)) { > + clk_div.prop_coef = 5; > + clk_div.int_coef = 11; > + clk_div.gain_ctl = 3; > + clk_div.targ_cnt = 9; > + if (best_clock.vco >= 4800000 && > + best_clock.vco < 5400000) > + clk_div.dcoampovr_en = 1; > + } else if (best_clock.vco == 5400000) { > + clk_div.prop_coef = 3; > + clk_div.int_coef = 8; > + clk_div.gain_ctl = 1; > + clk_div.targ_cnt = 9; > + } else { > + DRM_ERROR("Invalid VCO\n"); > + return false; > + } > + > if (clock > 270000) > clk_div.lanestagger = 0x18; > else if (clock > 135000) > @@ -1417,6 +1442,11 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > > crtc_state->dpll_hw_state.pll8 = clk_div.targ_cnt; > > + if (clk_div.dcoampovr_en) > + crtc_state->dpll_hw_state.pll10 = PORT_PLL_DCO_AMP_OVR_EN; > + > + crtc_state->dpll_hw_state.pll10 |= PORT_PLL_DCO_AMP(clk_div.dco_amp); > + > crtc_state->dpll_hw_state.pcsdw12 = > LANESTAGGER_STRAP_OVRD | clk_div.lanestagger; > > @@ -2348,10 +2378,19 @@ static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv, > temp |= pll->config.hw_state.pll8; > I915_WRITE(BXT_PORT_PLL(port, 8), temp); > > - /* > - * FIXME: program PORT_PLL_9/i_lockthresh according to the latest > - * specification update. > - */ > + /* Spec update: Write 5 to lock threshold */ > + temp = I915_READ(BXT_PORT_PLL(port, 9)); > + temp &= ~PORT_PLL_LOCK_THRESHOLD_MASK; > + temp |= (5 << 1); > + I915_WRITE(BXT_PORT_PLL(port, 9), temp); > + > + /* Spec update: Write DCO amplitude override value to i_dcoamp */ > + /* Spec update: Write DCO amplitude override enable to i_dcoampovrden_h */ > + temp = I915_READ(BXT_PORT_PLL(port, 10)); > + temp &= ~PORT_PLL_DCO_AMP_OVR_EN; > + temp &= ~PORT_PLL_DCO_AMP_MASK; > + temp |= pll->config.hw_state.pll10; > + I915_WRITE(BXT_PORT_PLL(port, 10), temp); > > /* Recalibrate with new settings */ > temp = I915_READ(BXT_PORT_PLL_EBB_4(port)); > @@ -2415,6 +2454,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, > hw_state->pll3 = I915_READ(BXT_PORT_PLL(port, 3)); > hw_state->pll6 = I915_READ(BXT_PORT_PLL(port, 6)); > hw_state->pll8 = I915_READ(BXT_PORT_PLL(port, 8)); > + hw_state->pll10 = I915_READ(BXT_PORT_PLL(port, 10)); > /* > * While we write to the group register to program all lanes at once we > * can read only lane registers. We configure all lanes the same way, so
On ma, 2015-05-04 at 20:50 +0530, Vandana Kannan wrote: > BUN 1: prop_coeff, int_coeff, tdctargetcnt programming updated and tied to > VCO frequencies. Program i_lockthresh in PORT_PLL_9. > > VCO calculated based on the formula: > Desired Output = Port bit rate in MHz (DisplayPort HBR2 is 5400 MHz) > Fast Clock = Desired Output / 2 > VCO = Fast Clock * P1 * P2 > > Prop_coeff, int_coeff, and tdctargetcnt modified according to above > calculation. > > BUN 2: Port PLLs require additional programming at certain frequencies - > DCO amplitude in PORT_PLL_10 > > Review comments from Siva which were addressed in the initial version of the > patch. > - Change PORT_PLL_LOCK_THRESHOLD to PORT_PLL_LOCK_THRESHOLD_MASK > - Calculate for HDMI > - Correct values for vco = 5.4 > - return in case of invalid vco range > > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 6 ++++ > drivers/gpu/drm/i915/intel_ddi.c | 72 +++++++++++++++++++++++++++++++--------- > 3 files changed, 63 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f3c77ca..cea54ee 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -309,7 +309,7 @@ struct intel_dpll_hw_state { > uint32_t cfgcr1, cfgcr2; > > /* bxt */ > - uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pcsdw12; > + uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pll10, pcsdw12; > }; > > struct intel_shared_dpll_config { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 36805b6..86fdc3e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1185,6 +1185,12 @@ enum skl_disp_power_wells { > #define PORT_PLL_GAIN_CTL(x) ((x) << 16) > /* PORT_PLL_8_A */ > #define PORT_PLL_TARGET_CNT_MASK 0x3FF > +/* PORT_PLL_9_A */ > +#define PORT_PLL_LOCK_THRESHOLD_MASK 0xe > +/* PORT_PLL_10_A */ > +#define PORT_PLL_DCO_AMP_OVR_EN (1<<27) This is actually OVR_EN_H, so I'd rename it accordingly, or simply use OVR_DIS. > +#define PORT_PLL_DCO_AMP_MASK 0x3c00 > +#define PORT_PLL_DCO_AMP(x) (x<<10) > #define _PORT_PLL_BASE(port) _PORT3(port, _PORT_PLL_0_A, \ > _PORT_PLL_0_B, \ > _PORT_PLL_0_C) > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 455d44b..3014c37 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1319,18 +1319,20 @@ struct bxt_clk_div { > uint32_t int_coef; > uint32_t gain_ctl; > uint32_t targ_cnt; > + uint32_t dcoampovr_en; As above _en_h, or _dis. > + uint32_t dco_amp; > uint32_t lanestagger; > }; > > /* pre-calculated values for DP linkrates */ > static struct bxt_clk_div bxt_dp_clk_val[7] = { > - /* 162 */ {4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, > - /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, > - /* 216 */ {3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 324 */ {4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, > - /* 432 */ {3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} > + /* 162 */ {4, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0xd}, > + /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0x18}, > + /* 216 */ {3, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 3, 9, 1, 15, 0xd}, > + /* 324 */ {4, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, > + /* 432 */ {3, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0x18} > }; > > static bool > @@ -1364,11 +1366,34 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > clk_div.m2_frac = best_clock.m2 & ((1 << 22) - 1); > clk_div.m2_frac_en = clk_div.m2_frac != 0; > > - /* FIXME: set coef, gain, targcnt based on freq band */ > - clk_div.prop_coef = 5; > - clk_div.int_coef = 11; > - clk_div.gain_ctl = 2; > - clk_div.targ_cnt = 9; > + clk_div.dco_amp = 15; > + clk_div.dcoampovr_en = 0; > + if (best_clock.vco >= 6200000 && best_clock.vco <= 6480000) { > + clk_div.prop_coef = 4; > + clk_div.int_coef = 9; > + clk_div.gain_ctl = 3; > + clk_div.targ_cnt = 8; > + } else if ((best_clock.vco > 5400000 && > + best_clock.vco < 6200000) || > + (best_clock.vco >= 4800000 && > + best_clock.vco < 5400000)) { > + clk_div.prop_coef = 5; > + clk_div.int_coef = 11; > + clk_div.gain_ctl = 3; > + clk_div.targ_cnt = 9; > + if (best_clock.vco >= 4800000 && > + best_clock.vco < 5400000) > + clk_div.dcoampovr_en = 1; > + } else if (best_clock.vco == 5400000) { > + clk_div.prop_coef = 3; > + clk_div.int_coef = 8; > + clk_div.gain_ctl = 1; > + clk_div.targ_cnt = 9; > + } else { > + DRM_ERROR("Invalid VCO\n"); > + return false; > + } > + This gets messy as we duplicate the code to calculate the PHY params for DP and HDMI and we do them in different ways. The lanestagger value for 324MHz symbol rate in bxt_dp_clk_val looks also incorrect, should be 0x18 instead of 0xd. Things would become much clearer if you'd use the above logic for both HDMI and DP: int vco; if (intel_encoder->type == INTEL_OUTPUT_HDMI) { ... vco = best_clock.vco; } else { ... vco = clock * 10 / 2 * clk_div.p1 * clk_div.p2; } Afterwards you can use the logic you added in this patch for both HDMI and DP. Similarly, lanestagger should be calculated in the same way for both HDMI and DP (changing it in a separate patch). > if (clock > 270000) > clk_div.lanestagger = 0x18; > else if (clock > 135000) > @@ -1417,6 +1442,11 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, > > crtc_state->dpll_hw_state.pll8 = clk_div.targ_cnt; > > + if (clk_div.dcoampovr_en) > + crtc_state->dpll_hw_state.pll10 = PORT_PLL_DCO_AMP_OVR_EN; > + > + crtc_state->dpll_hw_state.pll10 |= PORT_PLL_DCO_AMP(clk_div.dco_amp); > + > crtc_state->dpll_hw_state.pcsdw12 = > LANESTAGGER_STRAP_OVRD | clk_div.lanestagger; > > @@ -2348,10 +2378,19 @@ static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv, > temp |= pll->config.hw_state.pll8; > I915_WRITE(BXT_PORT_PLL(port, 8), temp); > > - /* > - * FIXME: program PORT_PLL_9/i_lockthresh according to the latest > - * specification update. > - */ > + /* Spec update: Write 5 to lock threshold */ This comment just says what the code does, so no need to spell it out separately. > + temp = I915_READ(BXT_PORT_PLL(port, 9)); > + temp &= ~PORT_PLL_LOCK_THRESHOLD_MASK; > + temp |= (5 << 1); > + I915_WRITE(BXT_PORT_PLL(port, 9), temp); > + > + /* Spec update: Write DCO amplitude override value to i_dcoamp */ > + /* Spec update: Write DCO amplitude override enable to i_dcoampovrden_h */ No need for the above comments. > + temp = I915_READ(BXT_PORT_PLL(port, 10)); > + temp &= ~PORT_PLL_DCO_AMP_OVR_EN; > + temp &= ~PORT_PLL_DCO_AMP_MASK; > + temp |= pll->config.hw_state.pll10; > + I915_WRITE(BXT_PORT_PLL(port, 10), temp); > > /* Recalibrate with new settings */ > temp = I915_READ(BXT_PORT_PLL_EBB_4(port)); > @@ -2415,6 +2454,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, > hw_state->pll3 = I915_READ(BXT_PORT_PLL(port, 3)); > hw_state->pll6 = I915_READ(BXT_PORT_PLL(port, 6)); > hw_state->pll8 = I915_READ(BXT_PORT_PLL(port, 8)); > + hw_state->pll10 = I915_READ(BXT_PORT_PLL(port, 10)); > /* > * While we write to the group register to program all lanes at once we > * can read only lane registers. We configure all lanes the same way, so We should also read out pll10 in bxt_ddi_pll_get_hw_state() for consistency. --Imre
On 5/5/2015 9:14 PM, Imre Deak wrote: > On ma, 2015-05-04 at 20:50 +0530, Vandana Kannan wrote: >> BUN 1: prop_coeff, int_coeff, tdctargetcnt programming updated and tied to >> VCO frequencies. Program i_lockthresh in PORT_PLL_9. >> >> VCO calculated based on the formula: >> Desired Output = Port bit rate in MHz (DisplayPort HBR2 is 5400 MHz) >> Fast Clock = Desired Output / 2 >> VCO = Fast Clock * P1 * P2 >> >> Prop_coeff, int_coeff, and tdctargetcnt modified according to above >> calculation. >> >> BUN 2: Port PLLs require additional programming at certain frequencies - >> DCO amplitude in PORT_PLL_10 >> >> Review comments from Siva which were addressed in the initial version of the >> patch. >> - Change PORT_PLL_LOCK_THRESHOLD to PORT_PLL_LOCK_THRESHOLD_MASK >> - Calculate for HDMI >> - Correct values for vco = 5.4 >> - return in case of invalid vco range >> >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> >> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 +- >> drivers/gpu/drm/i915/i915_reg.h | 6 ++++ >> drivers/gpu/drm/i915/intel_ddi.c | 72 +++++++++++++++++++++++++++++++--------- >> 3 files changed, 63 insertions(+), 17 deletions(-) >> [...] >> >> /* Recalibrate with new settings */ >> temp = I915_READ(BXT_PORT_PLL_EBB_4(port)); >> @@ -2415,6 +2454,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, >> hw_state->pll3 = I915_READ(BXT_PORT_PLL(port, 3)); >> hw_state->pll6 = I915_READ(BXT_PORT_PLL(port, 6)); >> hw_state->pll8 = I915_READ(BXT_PORT_PLL(port, 8)); >> + hw_state->pll10 = I915_READ(BXT_PORT_PLL(port, 10)); >> /* >> * While we write to the group register to program all lanes at once we >> * can read only lane registers. We configure all lanes the same way, so > > We should also read out pll10 in bxt_ddi_pll_get_hw_state() for > consistency. > I have added a readout above for pll10 or am I missing something ? Agree with all other comments, will resend the patch with the changes made. - Vandana > --Imre >
On ti, 2015-05-05 at 22:14 +0530, Kannan, Vandana wrote: > > On 5/5/2015 9:14 PM, Imre Deak wrote: > > On ma, 2015-05-04 at 20:50 +0530, Vandana Kannan wrote: > >> BUN 1: prop_coeff, int_coeff, tdctargetcnt programming updated and tied to > >> VCO frequencies. Program i_lockthresh in PORT_PLL_9. > >> > >> VCO calculated based on the formula: > >> Desired Output = Port bit rate in MHz (DisplayPort HBR2 is 5400 MHz) > >> Fast Clock = Desired Output / 2 > >> VCO = Fast Clock * P1 * P2 > >> > >> Prop_coeff, int_coeff, and tdctargetcnt modified according to above > >> calculation. > >> > >> BUN 2: Port PLLs require additional programming at certain frequencies - > >> DCO amplitude in PORT_PLL_10 > >> > >> Review comments from Siva which were addressed in the initial version of the > >> patch. > >> - Change PORT_PLL_LOCK_THRESHOLD to PORT_PLL_LOCK_THRESHOLD_MASK > >> - Calculate for HDMI > >> - Correct values for vco = 5.4 > >> - return in case of invalid vco range > >> > >> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> > >> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 2 +- > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++++ > >> drivers/gpu/drm/i915/intel_ddi.c | 72 +++++++++++++++++++++++++++++++--------- > >> 3 files changed, 63 insertions(+), 17 deletions(-) > >> > > [...] > > >> > >> /* Recalibrate with new settings */ > >> temp = I915_READ(BXT_PORT_PLL_EBB_4(port)); > >> @@ -2415,6 +2454,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, > >> hw_state->pll3 = I915_READ(BXT_PORT_PLL(port, 3)); > >> hw_state->pll6 = I915_READ(BXT_PORT_PLL(port, 6)); > >> hw_state->pll8 = I915_READ(BXT_PORT_PLL(port, 8)); > >> + hw_state->pll10 = I915_READ(BXT_PORT_PLL(port, 10)); > >> /* > >> * While we write to the group register to program all lanes at once we > >> * can read only lane registers. We configure all lanes the same way, so > > > > We should also read out pll10 in bxt_ddi_pll_get_hw_state() for > > consistency. > > > I have added a readout above for pll10 or am I missing something ? > Agree with all other comments, will resend the patch with the changes made. Ah sorry, I was blind. Ignore my comment about it. --Imre
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f3c77ca..cea54ee 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -309,7 +309,7 @@ struct intel_dpll_hw_state { uint32_t cfgcr1, cfgcr2; /* bxt */ - uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pcsdw12; + uint32_t ebb0, pll0, pll1, pll2, pll3, pll6, pll8, pll10, pcsdw12; }; struct intel_shared_dpll_config { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 36805b6..86fdc3e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1185,6 +1185,12 @@ enum skl_disp_power_wells { #define PORT_PLL_GAIN_CTL(x) ((x) << 16) /* PORT_PLL_8_A */ #define PORT_PLL_TARGET_CNT_MASK 0x3FF +/* PORT_PLL_9_A */ +#define PORT_PLL_LOCK_THRESHOLD_MASK 0xe +/* PORT_PLL_10_A */ +#define PORT_PLL_DCO_AMP_OVR_EN (1<<27) +#define PORT_PLL_DCO_AMP_MASK 0x3c00 +#define PORT_PLL_DCO_AMP(x) (x<<10) #define _PORT_PLL_BASE(port) _PORT3(port, _PORT_PLL_0_A, \ _PORT_PLL_0_B, \ _PORT_PLL_0_C) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 455d44b..3014c37 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1319,18 +1319,20 @@ struct bxt_clk_div { uint32_t int_coef; uint32_t gain_ctl; uint32_t targ_cnt; + uint32_t dcoampovr_en; + uint32_t dco_amp; uint32_t lanestagger; }; /* pre-calculated values for DP linkrates */ static struct bxt_clk_div bxt_dp_clk_val[7] = { - /* 162 */ {4, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, - /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0xd}, - /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0x18}, - /* 216 */ {3, 2, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, - /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 2, 9, 0xd}, - /* 324 */ {4, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0xd}, - /* 432 */ {3, 1, 32, 1677722, 1, 1, 5, 11, 2, 9, 0x18} + /* 162 */ {4, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, + /* 270 */ {4, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0xd}, + /* 540 */ {2, 1, 27, 0, 0, 1, 3, 8, 1, 9, 0, 15, 0x18}, + /* 216 */ {3, 2, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, + /* 243 */ {4, 1, 24, 1258291, 1, 1, 5, 11, 3, 9, 1, 15, 0xd}, + /* 324 */ {4, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0xd}, + /* 432 */ {3, 1, 32, 1677722, 1, 1, 4, 9, 3, 8, 0, 15, 0x18} }; static bool @@ -1364,11 +1366,34 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, clk_div.m2_frac = best_clock.m2 & ((1 << 22) - 1); clk_div.m2_frac_en = clk_div.m2_frac != 0; - /* FIXME: set coef, gain, targcnt based on freq band */ - clk_div.prop_coef = 5; - clk_div.int_coef = 11; - clk_div.gain_ctl = 2; - clk_div.targ_cnt = 9; + clk_div.dco_amp = 15; + clk_div.dcoampovr_en = 0; + if (best_clock.vco >= 6200000 && best_clock.vco <= 6480000) { + clk_div.prop_coef = 4; + clk_div.int_coef = 9; + clk_div.gain_ctl = 3; + clk_div.targ_cnt = 8; + } else if ((best_clock.vco > 5400000 && + best_clock.vco < 6200000) || + (best_clock.vco >= 4800000 && + best_clock.vco < 5400000)) { + clk_div.prop_coef = 5; + clk_div.int_coef = 11; + clk_div.gain_ctl = 3; + clk_div.targ_cnt = 9; + if (best_clock.vco >= 4800000 && + best_clock.vco < 5400000) + clk_div.dcoampovr_en = 1; + } else if (best_clock.vco == 5400000) { + clk_div.prop_coef = 3; + clk_div.int_coef = 8; + clk_div.gain_ctl = 1; + clk_div.targ_cnt = 9; + } else { + DRM_ERROR("Invalid VCO\n"); + return false; + } + if (clock > 270000) clk_div.lanestagger = 0x18; else if (clock > 135000) @@ -1417,6 +1442,11 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc, crtc_state->dpll_hw_state.pll8 = clk_div.targ_cnt; + if (clk_div.dcoampovr_en) + crtc_state->dpll_hw_state.pll10 = PORT_PLL_DCO_AMP_OVR_EN; + + crtc_state->dpll_hw_state.pll10 |= PORT_PLL_DCO_AMP(clk_div.dco_amp); + crtc_state->dpll_hw_state.pcsdw12 = LANESTAGGER_STRAP_OVRD | clk_div.lanestagger; @@ -2348,10 +2378,19 @@ static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv, temp |= pll->config.hw_state.pll8; I915_WRITE(BXT_PORT_PLL(port, 8), temp); - /* - * FIXME: program PORT_PLL_9/i_lockthresh according to the latest - * specification update. - */ + /* Spec update: Write 5 to lock threshold */ + temp = I915_READ(BXT_PORT_PLL(port, 9)); + temp &= ~PORT_PLL_LOCK_THRESHOLD_MASK; + temp |= (5 << 1); + I915_WRITE(BXT_PORT_PLL(port, 9), temp); + + /* Spec update: Write DCO amplitude override value to i_dcoamp */ + /* Spec update: Write DCO amplitude override enable to i_dcoampovrden_h */ + temp = I915_READ(BXT_PORT_PLL(port, 10)); + temp &= ~PORT_PLL_DCO_AMP_OVR_EN; + temp &= ~PORT_PLL_DCO_AMP_MASK; + temp |= pll->config.hw_state.pll10; + I915_WRITE(BXT_PORT_PLL(port, 10), temp); /* Recalibrate with new settings */ temp = I915_READ(BXT_PORT_PLL_EBB_4(port)); @@ -2415,6 +2454,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, hw_state->pll3 = I915_READ(BXT_PORT_PLL(port, 3)); hw_state->pll6 = I915_READ(BXT_PORT_PLL(port, 6)); hw_state->pll8 = I915_READ(BXT_PORT_PLL(port, 8)); + hw_state->pll10 = I915_READ(BXT_PORT_PLL(port, 10)); /* * While we write to the group register to program all lanes at once we * can read only lane registers. We configure all lanes the same way, so
BUN 1: prop_coeff, int_coeff, tdctargetcnt programming updated and tied to VCO frequencies. Program i_lockthresh in PORT_PLL_9. VCO calculated based on the formula: Desired Output = Port bit rate in MHz (DisplayPort HBR2 is 5400 MHz) Fast Clock = Desired Output / 2 VCO = Fast Clock * P1 * P2 Prop_coeff, int_coeff, and tdctargetcnt modified according to above calculation. BUN 2: Port PLLs require additional programming at certain frequencies - DCO amplitude in PORT_PLL_10 Review comments from Siva which were addressed in the initial version of the patch. - Change PORT_PLL_LOCK_THRESHOLD to PORT_PLL_LOCK_THRESHOLD_MASK - Calculate for HDMI - Correct values for vco = 5.4 - return in case of invalid vco range Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_reg.h | 6 ++++ drivers/gpu/drm/i915/intel_ddi.c | 72 +++++++++++++++++++++++++++++++--------- 3 files changed, 63 insertions(+), 17 deletions(-)