Re: [PATCH 1/8] drm/i915/icl: compute the TBT PLL registers

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jul 13, 2018 at 10:20:27AM -0700, Paulo Zanoni wrote:
> Em Qui, 2018-07-12 às 17:16 -0700, Rodrigo Vivi escreveu:
> > On Wed, Jul 11, 2018 at 02:59:02PM -0700, Paulo Zanoni wrote:
> > > Use the hardcoded tables provided by our spec.
> > > 
> > > v2:
> > >   - SSC stays disabled.
> > >   - Use intel_port_is_tc().
> > > 
> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 22 +++++++++++++++++++++-
> > >  1 file changed, 21 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > index b51ad2917dbe..7e5e6eb5dfe2 100644
> > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > @@ -2452,6 +2452,16 @@ static const struct skl_wrpll_params
> > > icl_dp_combo_pll_19_2MHz_values[] = {
> > >  	  .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0,
> > > .qdiv_ratio = 0},
> > >  };
> > >  
> > > +static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
> > > +	.dco_integer = 0x151, .dco_fraction = 0x4000,
> > 
> > I see 0x151 is the most common for 24MHz, but not the only one.
> > I also see 0x168 and 0x195, depending on the link rate.
> > 
> > So, what am I missing?
> 
> Are you looking at the table inside the Combo PLL section instead of
> the TBT PLL section? On that same page you are, scroll down by a lot
> until the "TBT PLL Divider Values" table inside the TBT section.

Ha! I knew I was missing something.

Could the order be inverted to match the spec order?
first line 19.2
second 24

> 
> 
> > 
> > > +	.pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0,
> > > .qdiv_ratio = 0,

Also, I see pdiv = 5 and and qdiv = 1...

but I'm sure that I'm missing something else again...
probably some mathmagic somewhere... where?

anyways, if that is the case could we change to this table
here to match spec one?

Thanks,
Rodrigo.

> > > +};
> > > +
> > > +static const struct skl_wrpll_params icl_tbt_pll_19_2MHz_values =
> > > {
> > > +	.dco_integer = 0x1A5, .dco_fraction = 0x7000,
> > > +	.pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0,
> > > .qdiv_ratio = 0,
> > > +};
> > > +
> > >  static bool icl_calc_dp_combo_pll(struct drm_i915_private
> > > *dev_priv, int clock,
> > >  				  struct skl_wrpll_params
> > > *pll_params)
> > >  {
> > > @@ -2494,6 +2504,14 @@ static bool icl_calc_dp_combo_pll(struct
> > > drm_i915_private *dev_priv, int clock,
> > >  	return true;
> > >  }
> > >  
> > > +static bool icl_calc_tbt_pll(struct drm_i915_private *dev_priv,
> > > int clock,
> > > +			     struct skl_wrpll_params *pll_params)
> > > +{
> > > +	*pll_params = dev_priv->cdclk.hw.ref == 24000 ?
> > > +			icl_tbt_pll_24MHz_values :
> > > icl_tbt_pll_19_2MHz_values;
> > > +	return true;
> > > +}
> > > +
> > >  static bool icl_calc_dpll_state(struct intel_crtc_state
> > > *crtc_state,
> > >  				struct intel_encoder *encoder, int
> > > clock,
> > >  				struct intel_dpll_hw_state
> > > *pll_state)
> > > @@ -2503,7 +2521,9 @@ static bool icl_calc_dpll_state(struct
> > > intel_crtc_state *crtc_state,
> > >  	struct skl_wrpll_params pll_params = { 0 };
> > >  	bool ret;
> > >  
> > > -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > > +	if (intel_port_is_tc(dev_priv, encoder->port))
> > > +		ret = icl_calc_tbt_pll(dev_priv, clock,
> > > &pll_params);
> > > +	else if (intel_crtc_has_type(crtc_state,
> > > INTEL_OUTPUT_HDMI))
> > >  		ret = cnl_ddi_calculate_wrpll(clock, dev_priv,
> > > &pll_params);
> > >  	else
> > >  		ret = icl_calc_dp_combo_pll(dev_priv, clock,
> > > &pll_params);
> > > -- 
> > > 2.14.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx