On Mon, Jun 05, 2017 at 05:59:02PM +0000, Vivi, Rodrigo wrote:
> On Fri, 2017-06-02 at 21:06 +0300, Imre Deak wrote:
> > On Thu, Apr 06, 2017 at 12:15:24PM -0700, Rodrigo Vivi wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > Add support for reading out the cdclk frequency from the hardware on
> > > CNL. Very similar to BXT, with a few new twists and turns:
> > > * the PLL is now called CDCLK PLL, not DE PLL
> > > * reference clock can be 24 MHz in addition to the 19.2 MHz BXT had
> > > * the ratio now lives in the PLL enable register
> > > * Only 1x and 2x CD2X dividers are supported
> > >
> > > v2: Deal with PLL lock bit the same way as BXT/SKL do now
> > > v3: DSSM refclk indicator is bit 31 not 24 (Ander)
> > > v4: Rebased by Rodrigo after Ville's cdclk rework.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++
> > > drivers/gpu/drm/i915/intel_cdclk.c | 54 +++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 58 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index ac8a223..8353892 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6557,6 +6557,9 @@ enum {
> > > #define SKL_DFSM_PIPE_B_DISABLE (1 << 21)
> > > #define SKL_DFSM_PIPE_C_DISABLE (1 << 28)
> > >
> > > +#define SKL_DSSM _MMIO(0x51004)
> > > +#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz (1 << 31)
> > > +
> > > #define GEN7_FF_SLICE_CS_CHICKEN1 _MMIO(0x20e0)
> > > #define GEN9_FFSC_PERCTX_PREEMPT_CTRL (1<<14)
> > >
> > > @@ -8130,6 +8133,8 @@ enum {
> > > #define BXT_DE_PLL_ENABLE _MMIO(0x46070)
> > > #define BXT_DE_PLL_PLL_ENABLE (1 << 31)
> > > #define BXT_DE_PLL_LOCK (1 << 30)
> > > +#define CNL_CDCLK_PLL_RATIO(x) (x) /* {28,44} * 19.2 or 24MHz */
>
> >
> > Nit: 35,55 are also valid for 19.2MHz.
>
> hm... should I just remove the comments then?!
>
>
> > > +#define CNL_CDCLK_PLL_RATIO_MASK 0xff
> > >
> > > /* GEN9 DC */
> > > #define DC_STATE_EN _MMIO(0x45504)
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 4745596..a4e2bd5 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1400,6 +1400,56 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> > > bxt_set_cdclk(dev_priv, &cdclk_state);
> > > }
> > >
> > > +static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv,
> > > + struct intel_cdclk_state *cdclk_state)
> > > +{
> > > + u32 val;
> > > +
> > > + if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
> > > + cdclk_state->ref = 24000;
> > > + else
> > > + cdclk_state->ref = 19200;
> > > +
> > > + cdclk_state->vco = 0;
> > > +
> > > + val = I915_READ(BXT_DE_PLL_ENABLE);
> > > + if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
> > > + return;
> > > +
> > > + if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
> > > + return;
> > > +
> > > + cdclk_state->vco = (val & CNL_CDCLK_PLL_RATIO_MASK) * cdclk_state->ref;
> > > +}
> > > +
> > > +static void cnl_get_cdclk(struct drm_i915_private *dev_priv,
> > > + struct intel_cdclk_state *cdclk_state)
> > > +{
> > > + u32 divider;
> > > + int div;
> > > +
> > > + cnl_cdclk_pll_update(dev_priv, cdclk_state);
> >
> > The other platforms set cdclk to the ref clock here, not sure
> > if it's ok to leave it uninited. With that change it looks ok:
>
> Not sure how to address this here...
> I see bxt and skl using the cdclk_state here...
Assuming refclk is the bypass clock then just doing what the earlier
platforms do would be correct. IIRC there was some platform where
the bypass clock wasn't the refclk, but that was perhaps some
future thing. Either way, whatever the bypass clock is we will want
the readout to correctly reflect it when the PLL is off.
>
> >
> > Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> >
> > > +
> > > + if (cdclk_state->vco == 0)
> > > + return;
> > > +
> > > + divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
> > > +
> > > + switch (divider) {
> > > + case BXT_CDCLK_CD2X_DIV_SEL_1:
> > > + div = 2;
> > > + break;
> > > + case BXT_CDCLK_CD2X_DIV_SEL_2:
> > > + div = 4;
> > > + break;
> > > + default:
> > > + MISSING_CASE(divider);
> > > + return;
> > > + }
> > > +
> > > + cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div);
> > > +}
> > > +
> > > /**
> > > * intel_cdclk_state_compare - Determine if two CDCLK states differ
> > > * @a: first CDCLK state
> > > @@ -1897,7 +1947,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> > > skl_modeset_calc_cdclk;
> > > }
> > >
> > > - if (IS_GEN9_BC(dev_priv))
> > > + if (IS_CANNONLAKE(dev_priv))
> > > + dev_priv->display.get_cdclk = cnl_get_cdclk;
> > > + else if (IS_GEN9_BC(dev_priv))
> > > dev_priv->display.get_cdclk = skl_get_cdclk;
> > > else if (IS_GEN9_LP(dev_priv))
> > > dev_priv->display.get_cdclk = bxt_get_cdclk;
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
