[05/10] drm/i915: Disable CLKOUT_DP bending on LPT/WPT as needed

Submitted by Ville Syrjälä on Dec. 1, 2015, 1:08 p.m.

Details

Message ID 1448975321-20192-6-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show
Series "HSW/BDW PCH modeset fixes and stuff" ( rev: 6 5 4 3 2 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Ville Syrjälä Dec. 1, 2015, 1:08 p.m.
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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.
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@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

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)
+{
+	uint32_t tmp;
+
+	int idx = BEND_IDX(steps);
+
+	if (WARN_ON(idx >= ARRAY_SIZE(sscdivintphase)))
+		return;
+
+	mutex_lock(&dev_priv->sb_lock);
+
+	if (steps % 5 != 0)
+		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);
+	}
 }
 
 /*

Comments

2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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.
> 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@linux.intel.com>
> ---
>  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?

> +{
> +       uint32_t tmp;
> +
> +       int idx = BEND_IDX(steps);

So shouldn't we do the following?

if (WARN_ON(steps % 5 != 0))
        return;

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

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Dec 02, 2015 at 11:35:00AM -0200, Paulo Zanoni wrote:
> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 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@linux.intel.com>
> > ---
> >  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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
2015-12-03 8:03 GMT-02:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Wed, Dec 02, 2015 at 11:35:00AM -0200, Paulo Zanoni wrote:
>> 2015-12-01 11:08 GMT-02:00  <ville.syrjala@linux.intel.com>:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > 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@linux.intel.com>
>> > ---
>> >  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.

I actually liked your approach since it kinda matches the spec, so you
can keep it. I was just asking for confirmation due to the "% 5"
problem below.

>
>>
>> > +{
>> > +       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@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC