[Intel-gfx] [PATCH] drm/i915/tgl+: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Jul 17 18:17:05 UTC 2020
On Fri, Jul 17, 2020 at 08:37:23PM +0300, Imre Deak wrote:
> On Fri, Jul 17, 2020 at 07:10:39PM +0300, Ville Syrjälä wrote:
> > On Tue, Jul 14, 2020 at 07:32:36PM +0300, Imre Deak wrote:
> > > Apply Display WA #22010492432 for combo PHY PLLs too. This should fix a
> > > problem where the PLL output frequency is slightly off with the current
> > > PLL fractional divider value.
> > >
> > > I haven't seen an actual case where this causes a problem, but let's
> > > follow the spec. It's also needed on some EHL platforms, but for that we
> > > also need a way to distinguish the affected EHL SKUs, so I leave that
> > > for a follow-up.
> > >
> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 54 +++++++++++++++++--
> > > 1 file changed, 49 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > index aeb6ee395cce..aee9101f5e7c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > @@ -2887,7 +2887,7 @@ static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
> > > };
> > >
> > >
> > > -/* Also used for 38.4 MHz values. */
> > > +/* On ICL also used for 38.4 MHz values. */
> > > static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
> > > { 540000,
> > > { .dco_integer = 0x1A5, .dco_fraction = 0x7000, /* [0]: 5.4 */
> > > @@ -2915,6 +2915,37 @@ static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
> > > .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > };
> > >
> > > +/*
> > > + * Display WA #22010492432: tgl
> > > + * Divide the nominal .dco_fraction value by 2.
> > > + */
> > > +static const struct icl_combo_pll_params tgl_dp_combo_pll_38_4MHz_values[] = {
> > > + { 540000,
> > > + { .dco_integer = 0x1A5, .dco_fraction = 0x3800, /* [0]: 5.4 */
> > > + .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > + { 270000,
> > > + { .dco_integer = 0x1A5, .dco_fraction = 0x3800, /* [1]: 2.7 */
> > > + .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > + { 162000,
> > > + { .dco_integer = 0x1A5, .dco_fraction = 0x3800, /* [2]: 1.62 */
> > > + .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > + { 324000,
> > > + { .dco_integer = 0x1A5, .dco_fraction = 0x3800, /* [3]: 3.24 */
> > > + .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > + { 216000,
> > > + { .dco_integer = 0x1C2, .dco_fraction = 0x0000, /* [4]: 2.16 */
> > > + .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> > > + { 432000,
> > > + { .dco_integer = 0x1C2, .dco_fraction = 0x0000, /* [5]: 4.32 */
> > > + .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > + { 648000,
> > > + { .dco_integer = 0x1FA, .dco_fraction = 0x1000, /* [6]: 6.48 */
> > > + .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > > + { 810000,
> > > + { .dco_integer = 0x1A5, .dco_fraction = 0x3800, /* [7]: 8.1 */
> > > + .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,
> > > .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0,
> > > @@ -2950,13 +2981,26 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> > > struct skl_wrpll_params *pll_params)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> > > - const struct icl_combo_pll_params *params =
> > > - dev_priv->dpll.ref_clks.nssc == 24000 ?
> > > - icl_dp_combo_pll_24MHz_values :
> > > - icl_dp_combo_pll_19_2MHz_values;
> > > + const struct icl_combo_pll_params *params;
> > > int clock = crtc_state->port_clock;
> > > int i;
> > >
> > > + switch (dev_priv->dpll.ref_clks.nssc) {
> > > + case 19200:
> > > + params = icl_dp_combo_pll_19_2MHz_values;
> > > + break;
> > > + case 24000:
> > > + params = icl_dp_combo_pll_24MHz_values;
> > > + break;
> > > + case 38400:
> > > + /* TODO: Apply WA #22010492432 on EHL too. */
> > > + if (IS_TIGERLAKE(dev_priv))
> > > + params = tgl_dp_combo_pll_38_4MHz_values;
> > > + else
> > > + params = icl_dp_combo_pll_19_2MHz_values;
> > > + break;
> > > + }
> >
> > Hmm. This only takes care of DP AFAICS. What about HDMI?
> > Also doesn't readout need some tweaking too?
>
> Err, I missed those, thanks for catching it.
>
> > Should we rather consider pushing this w/a deeper into
> > the register progamming/readout code?
>
> Yes, would be better to have it in a single spot. Could you confirm if
> icl_calc_dpll_state() and __cnl_ddi_wrpll_get_freq() is the right place
> for those, while relying on tbt returning the correct fixed freq for the
> dot clock readout?
Sounds about right.
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list