On Wed, Dec 02, 2015 at 11:35:00AM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00 <ville.syrjala@xxxxxxxxxxxxxxx>:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > When we want to use SPLL for FDI we want SSC, which means we have to
> > disable clock bending for the PCH SSC reference (bend and spread are
> > mtutually exclusive). So let's turn off bending when we want spread.
^^^^^^^^^
Spotted this typo here just now. Will fix that one too.
> > In case the BIOS enabled clock bending for some reason we'll just turn
> > it off and enable the spread mode instead.
> >
> > Not sure what happens if the BIOS is actually using the bend source for
> > HDMI at this time, but I suppose it should be no worse than what already
> > happens when we simply turn on the spread.
> >
> > We don't currently use the bend source for anything, and only use the
> > PCH SSC reference for the SPLL to drive FDI (always with spread).
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++++++++++++++++++++++--
> > 2 files changed, 65 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 6d7ac192982d..fcc819f400a6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7320,6 +7320,7 @@ enum skl_disp_power_wells {
> > #define SBI_READY (0x0<<0)
> >
> > /* SBI offsets */
> > +#define SBI_SSCDIVINTPHASE 0x0200
> > #define SBI_SSCDIVINTPHASE6 0x0600
> > #define SBI_SSCDIVINTPHASE_DIVSEL_MASK ((0x7f)<<1)
> > #define SBI_SSCDIVINTPHASE_DIVSEL(x) ((x)<<1)
> > @@ -7327,6 +7328,7 @@ enum skl_disp_power_wells {
> > #define SBI_SSCDIVINTPHASE_INCVAL(x) ((x)<<8)
> > #define SBI_SSCDIVINTPHASE_DIR(x) ((x)<<15)
> > #define SBI_SSCDIVINTPHASE_PROPAGATE (1<<0)
> > +#define SBI_SSCDITHPHASE 0x0204
> > #define SBI_SSCCTL 0x020c
> > #define SBI_SSCCTL6 0x060C
> > #define SBI_SSCCTL_PATHALT (1<<3)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d049b087e8e6..c429029e3bed 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8551,6 +8551,65 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
> > mutex_unlock(&dev_priv->sb_lock);
> > }
> >
> > +#define BEND_IDX(steps) ((50 + (steps)) / 5)
> > +
> > +static const uint16_t sscdivintphase[] = {
> > + [BEND_IDX( 50)] = 0x3B23,
> > + [BEND_IDX( 45)] = 0x3B23,
> > + [BEND_IDX( 40)] = 0x3C23,
> > + [BEND_IDX( 35)] = 0x3C23,
> > + [BEND_IDX( 30)] = 0x3D23,
> > + [BEND_IDX( 25)] = 0x3D23,
> > + [BEND_IDX( 20)] = 0x3E23,
> > + [BEND_IDX( 15)] = 0x3E23,
> > + [BEND_IDX( 10)] = 0x3F23,
> > + [BEND_IDX( 5)] = 0x3F23,
> > + [BEND_IDX( 0)] = 0x0025,
> > + [BEND_IDX( -5)] = 0x0025,
> > + [BEND_IDX(-10)] = 0x0125,
> > + [BEND_IDX(-15)] = 0x0125,
> > + [BEND_IDX(-20)] = 0x0225,
> > + [BEND_IDX(-25)] = 0x0225,
> > + [BEND_IDX(-30)] = 0x0325,
> > + [BEND_IDX(-35)] = 0x0325,
> > + [BEND_IDX(-40)] = 0x0425,
> > + [BEND_IDX(-45)] = 0x0425,
> > + [BEND_IDX(-50)] = 0x0525,
> > +};
> > +
> > +/*
> > + * Bend CLKOUT_DP
> > + * steps -50 to 50 inclusive, in steps of 5
> > + * < 0 slow down the clock, > 0 speed up the clock, 0 == no bend (135MHz)
> > + * change in clock period = -(steps / 10) * 5.787 ps
> > + */
> > +static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv, int steps)
>
> As far as I understood from your comments and the table, "int steps"
> should always be a multiple of 5, right?
Yes, it's in decimal .1 fixed point, but only .0 and .5 values are allowed.
I could have made it binary .1 fixed point, but then comparing with the
spec might have been somewhat less obvious.
>
> > +{
> > + uint32_t tmp;
> > +
> > + int idx = BEND_IDX(steps);
>
> So shouldn't we do the following?
>
> if (WARN_ON(steps % 5 != 0))
> return;
Yes, that's a good idea.
>
> > +
> > + if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
> > + return;
> > +
> > + mutex_lock(&dev_priv->sb_lock);
> > +
> > + if (steps % 5 != 0)
>
> Shouldn't this be "steps % 10 != 0"? Otherwise, we'll always assign 0x0.
Doh! Thanks for catching that. Will fix.
>
> Everything else looks correct.
>
>
> > + tmp = 0xAAAAAAAB;
> > + else
> > + tmp = 0x00000000;
> > + intel_sbi_write(dev_priv, SBI_SSCDITHPHASE, tmp, SBI_ICLK);
> > +
> > + tmp = intel_sbi_read(dev_priv, SBI_SSCDIVINTPHASE, SBI_ICLK);
> > + tmp &= 0xffff0000;
> > + tmp |= sscdivintphase[idx];
> > + intel_sbi_write(dev_priv, SBI_SSCDIVINTPHASE, tmp, SBI_ICLK);
> > +
> > + mutex_unlock(&dev_priv->sb_lock);
> > +}
> > +
> > +#undef BEND_IDX
> > +
> > static void lpt_init_pch_refclk(struct drm_device *dev)
> > {
> > struct intel_encoder *encoder;
> > @@ -8566,10 +8625,12 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
> > }
> > }
> >
> > - if (has_vga)
> > + if (has_vga) {
> > + lpt_bend_clkout_dp(to_i915(dev), 0);
> > lpt_enable_clkout_dp(dev, true, true);
> > - else
> > + } else {
> > lpt_disable_clkout_dp(dev);
> > + }
> > }
> >
> > /*
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
