drm/i915/bxt: Port PLL programming BUN
diff mbox

Message ID 1430752821-12428-1-git-send-email-vandana.kannan@intel.com
State New
Headers show

Commit Message

vandana.kannan@intel.com May 4, 2015, 3:20 p.m. UTC
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(-)

Comments

Shuang He May 5, 2015, 12:13 a.m. UTC | #1
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 '*'
Sivakumar Thulasimani May 5, 2015, 4:53 a.m. UTC | #2
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
Imre Deak May 5, 2015, 3:44 p.m. UTC | #3
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
vandana.kannan@intel.com May 5, 2015, 4:44 p.m. UTC | #4
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
>
Imre Deak May 5, 2015, 4:49 p.m. UTC | #5
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

Patch
diff mbox

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