Re: [PATCH 2/2] drm/i915/icl: Get DDI clock for ICL for MG PLL and TBT PLL

["Souza, Jose" <jose.souza@intel.com>]

On Fri, 2018-07-27 at 13:04 -0700, Paulo Zanoni wrote:
> From: Manasi Navare <manasi.d.navare@intel.com>
> 
> PLLs are the source clocks for the DDIs so in order to determine the
> ddi clock we need to check the PLL configuration.
> 
> For MG PHy Ports (C - F), depending on whether it is a TBT PLL or MG
> PLL the link lock can be obtained from the the PLL divisors based on
> the specification.
> 
> v2 (from Paulo):
>  * Make the algorithm look more like what's in the spec, also
> document
>    where we differ form the spec and why.
>  * Make the code a little more consistent with our coding style.
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  5 ++
>  drivers/gpu/drm/i915/intel_ddi.c | 81
> +++++++++++++++++++++++++++++++-
>  2 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index e04ac47d53db..cd37e76d4d19 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9458,6 +9458,7 @@ enum skl_power_gate {
>  #define   MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5		(2 << 12)
>  #define   MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7		(3 << 12)
>  #define   MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(x)		((x) << 8)
> +#define   MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT		8
>  #define   MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK		(0xf <<
> 8)
>  #define MG_CLKTOP2_HSCLKCTL(port) _MMIO_PORT((port) - PORT_C, \
>  					     _MG_CLKTOP2_HSCLKCTL_PORT1
> , \
> @@ -9468,7 +9469,10 @@ enum skl_power_gate {
>  #define _MG_PLL_DIV0_PORT3				0x16AA00
>  #define _MG_PLL_DIV0_PORT4				0x16BA00
>  #define   MG_PLL_DIV0_FRACNEN_H				(1 <<
> 30)
> +#define   MG_PLL_DIV0_FBDIV_FRAC_MASK			(0x3fff
> ff << 8)
> +#define   MG_PLL_DIV0_FBDIV_FRAC_SHIFT			8
>  #define   MG_PLL_DIV0_FBDIV_FRAC(x)			((x) << 8)
> +#define   MG_PLL_DIV0_FBDIV_INT_MASK			(0xff << 0)
>  #define   MG_PLL_DIV0_FBDIV_INT(x)			((x) << 0)
>  #define MG_PLL_DIV0(port) _MMIO_PORT((port) - PORT_C,
> _MG_PLL_DIV0_PORT1, \
>  				     _MG_PLL_DIV0_PORT2)
> @@ -9483,6 +9487,7 @@ enum skl_power_gate {
>  #define   MG_PLL_DIV1_DITHER_DIV_4			(2 << 12)
>  #define   MG_PLL_DIV1_DITHER_DIV_8			(3 << 12)
>  #define   MG_PLL_DIV1_NDIVRATIO(x)			((x) << 4)
> +#define   MG_PLL_DIV1_FBPREDIV_MASK			(0xf << 0)
>  #define   MG_PLL_DIV1_FBPREDIV(x)			((x) << 0)
>  #define MG_PLL_DIV1(port) _MMIO_PORT((port) - PORT_C,
> _MG_PLL_DIV1_PORT1, \
>  				     _MG_PLL_DIV1_PORT2)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0adc043529f2..a18d57046bcc 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1427,6 +1427,81 @@ static int cnl_calc_wrpll_link(struct
> drm_i915_private *dev_priv,
>  	return dco_freq / (p0 * p1 * p2 * 5);
>  }
>  
> +static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv,
> +				 enum port port)
> +{
> +	u32 val = I915_READ(DDI_CLK_SEL(port)) & DDI_CLK_SEL_MASK;
> +
> +	switch (val) {
> +	case DDI_CLK_SEL_NONE:
> +		return 0;
> +	case DDI_CLK_SEL_TBT_162:
> +		return 162000;
> +	case DDI_CLK_SEL_TBT_270:
> +		return 270000;
> +	case DDI_CLK_SEL_TBT_540:
> +		return 540000;
> +	case DDI_CLK_SEL_TBT_810:
> +		return 810000;
> +	default:
> +		MISSING_CASE(val);
> +		return 0;
> +	}
> +}
> +
> +static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv,
> +				enum port port)
> +{
> +	u32 mg_pll_div0, mg_clktop_hsclkctl;
> +	u32 m1, m2_int, m2_frac, div1, div2, refclk;
> +	u64 tmp;
> +
> +	refclk = dev_priv->cdclk.hw.ref;
> +
> +	mg_pll_div0 = I915_READ(MG_PLL_DIV0(port));
> +	mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(port));
> +
> +	m1 = I915_READ(MG_PLL_DIV1(port)) & MG_PLL_DIV1_FBPREDIV_MASK;
> +	m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
> +	m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ?
> +		  (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
> +		  MG_PLL_DIV0_FBDIV_FRAC_SHIFT : 0;
> +
> +	switch (mg_clktop_hsclkctl &
> MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
> +	case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2:
> +		div1 = 2;
> +		break;
> +	case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_3:
> +		div1 = 3;
> +		break;
> +	case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5:
> +		div1 = 5;
> +		break;
> +	case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7:
> +		div1 = 7;
> +		break;
> +	default:
> +		MISSING_CASE(mg_clktop_hsclkctl);
> +		return 0;
> +	}
> +
> +	div2 = (mg_clktop_hsclkctl &
> MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK) >>
> +		MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT;
> +	/* div2 value of 0 is same as 1 means no div */
> +	if (div2 == 0)
> +		div2 = 1;
> +
> +	/*
> +	 * Adjust the original formula to delay the division by 2^22 in
> order to
> +	 * minimize possible rounding errors.
> +	 */
> +	tmp = (u64)m1 * m2_int * refclk +
> +	      (((u64)m1 * m2_frac * refclk) >> 22);

In my calculations this is not the same as: M1 * M2 * refclk

As M2 is = m2_int + (m2_frac/2^22)

Other than that everything else looks good.


> +	tmp = div_u64(tmp, 5 * div1 * div2);
> +
> +	return tmp;
> +}
> +
>  static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
>  {
>  	int dotclock;
> @@ -1467,8 +1542,10 @@ static void icl_ddi_clock_get(struct
> intel_encoder *encoder,
>  			link_clock =
> icl_calc_dp_combo_pll_link(dev_priv,
>  								pll_id)
> ;
>  	} else {
> -		/* FIXME - Add for MG PLL */
> -		WARN(1, "MG PLL clock_get code not implemented yet\n");
> +		if (pll_id == DPLL_ID_ICL_TBTPLL)
> +			link_clock = icl_calc_tbt_pll_link(dev_priv,
> port);
> +		else
> +			link_clock = icl_calc_mg_pll_link(dev_priv,
> port);
>  	}
>  
>  	pipe_config->port_clock = link_clock;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx