[Intel-gfx] [PATCH 1/8] drm/i915/icl: compute the TBT PLL registers

[Paulo Zanoni]

Em Sex, 2018-07-13 às 11:04 -0700, Rodrigo Vivi escreveu:
> 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 at intel.com>
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at 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

The combo table has 24 first (both in BSpec and in the code), so this
patch matches the order that's already in our code. Also the code
checks usually put 24000 first (since "else" fits 19200 and 38400), so
I kept the "24000 first" pattern. But I can paint it with the other
color if you insist :).


> 
> > 
> > 
> > > 
> > > > +	.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?

Look at the DPLL_CFGCR1 definition (page 15725).

Bit 9 value 0 means Q divider = 1.

Bits 8:6 value 100b means 3.

Bits 5:2 value 0100b means 5, 1000b means 7.

Amazing, huh?

See the combo pll table: it includes both the bit numbers and the
"original" numbers. The TBT one doesn't. That's very confusing, I know.
Our table is supposed to have the bits to add to the registers, with
the "real numbers" in comments. This also matches the combo tables
defined just above.

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

I would prefer to keep as-is since otherwise we'd have to add code to
translate from the table numbers to register bit numbers.

> 
> 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 at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx