diff mbox

[2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.

Message ID 1380267060-29244-1-git-send-email-chon.ming.lee@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chon Ming Lee Sept. 27, 2013, 7:31 a.m. UTC
CDCLK is used to generate the gmbus clock.  This is normally done by
BIOS. Program the value if the BIOS-less system doesn't do it.

v2: Move this to intel_i2c_reset to allow reprogram the gmbus frequency
during resume. (Daniel)

v3: Change GMBUS_FREQ to GMBUSFREQ_VLV, and use VLV_DISPLAY_BASE.
(Ville).
	Remove cdclk_ratio[] table, and calculate the cdclk ratio instead.
(Ville).
 	Change the shift then mask for reg read, to mask first, then shift.
(Ville).
	Remove the gmbus frequency calculation = cdclk/1.01.  Based on BIOS
programming, gmbus frequency = cdclk frequency. (Ville)
	Add get_disp_clk_div, which can use to get cdclk/czclk divide.

v4: Fix the mmio_offset base for CZCLK_CDCLK_FREQ_RATIO, gmbus_freq
calculation, and duplicate check for gmbus_freq. (Ville)

In VLV, the spec is wrong about 4Mhz reference frequency for GMBUS. It
should be 1Mhz.

Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |    8 +++++
 drivers/gpu/drm/i915/intel_i2c.c |   57 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 0 deletions(-)

Comments

Ville Syrjälä Sept. 27, 2013, 10:07 a.m. UTC | #1
On Fri, Sep 27, 2013 at 03:31:00PM +0800, Chon Ming Lee wrote:
> CDCLK is used to generate the gmbus clock.  This is normally done by
> BIOS. Program the value if the BIOS-less system doesn't do it.
> 
> v2: Move this to intel_i2c_reset to allow reprogram the gmbus frequency
> during resume. (Daniel)
> 
> v3: Change GMBUS_FREQ to GMBUSFREQ_VLV, and use VLV_DISPLAY_BASE.
> (Ville).
> 	Remove cdclk_ratio[] table, and calculate the cdclk ratio instead.
> (Ville).
>  	Change the shift then mask for reg read, to mask first, then shift.
> (Ville).
> 	Remove the gmbus frequency calculation = cdclk/1.01.  Based on BIOS
> programming, gmbus frequency = cdclk frequency. (Ville)
> 	Add get_disp_clk_div, which can use to get cdclk/czclk divide.
> 
> v4: Fix the mmio_offset base for CZCLK_CDCLK_FREQ_RATIO, gmbus_freq
> calculation, and duplicate check for gmbus_freq. (Ville)
> 
> In VLV, the spec is wrong about 4Mhz reference frequency for GMBUS. It
> should be 1Mhz.
> 
> Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |    8 +++++
>  drivers/gpu/drm/i915/intel_i2c.c |   57 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a0a9811..b37dfd8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -391,6 +391,8 @@
>  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
>  
>  /* vlv2 north clock has */
> +#define CCK_FUSE_REG				0x8
> +#define  CCK_FUSE_HPLL_FREQ_MASK		0x3
>  #define CCK_REG_DSI_PLL_FUSE			0x44
>  #define CCK_REG_DSI_PLL_CONTROL			0x48
>  #define  DSI_PLL_VCO_EN				(1 << 31)
> @@ -1438,6 +1440,12 @@
>  
>  #define MI_ARB_VLV		(VLV_DISPLAY_BASE + 0x6504)
>  
> +#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
> +#define   CDCLK_FREQ_SHIFT	4
> +#define   CDCLK_FREQ_MASK	(0x1f << CDCLK_FREQ_SHIFT)
> +#define   CZCLK_FREQ_MASK	0xf
> +#define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
> +
>  /*
>   * Palette regs
>   */
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index d1c1e0f7..b579ffd 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -34,6 +34,11 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  
> +enum disp_clk {
> +	CDCLK,
> +	CZCLK
> +};
> +
>  struct gmbus_port {
>  	const char *name;
>  	int reg;
> @@ -58,10 +63,62 @@ to_intel_gmbus(struct i2c_adapter *i2c)
>  	return container_of(i2c, struct intel_gmbus, adapter);
>  }
>  
> +static int get_disp_clk_div(struct drm_i915_private *dev_priv, enum disp_clk clk)
> +{
> +	u32 reg_val;
> +	int clk_ratio;
> +
> +	reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
> +
> +	if (clk == CDCLK)
> +		clk_ratio = ((reg_val & CDCLK_FREQ_MASK) >> CDCLK_FREQ_SHIFT) + 1;
> +	else
> +		clk_ratio = (reg_val & CZCLK_FREQ_MASK) + 1;
> +
> +	return clk_ratio;
> +}
> +
> +static void gmbus_set_freq(struct drm_i915_private *dev_priv)
> +{
> +	int vco_freq[] = { 800, 1600, 2000, 2400 };
> +	int gmbus_freq = 0, cdclk_div, hpll_freq;
> +
> +	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
> +
> +	/* Skip setting the gmbus freq if BIOS has already programmed it */
> +	if (I915_READ(GMBUSFREQ_VLV) != 0xA0)
> +		return;
> +
> +	/* Obtain SKU information */
> +	mutex_lock(&dev_priv->dpio_lock);
> +	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) & CCK_FUSE_HPLL_FREQ_MASK;
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	/* Get the CDCLK divide ratio */
> +	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
> +
> +	/* Program the gmbus_freq based on the cdclk frequency */
> +	if (cdclk_div)
> +		gmbus_freq = (vco_freq[hpll_freq] << 1) / cdclk_div;


OK based on the information that you dug up that we do need to aim for
1MHz GMBUS freq, the patch looks good to me. I think we should add a
comment here about the 1MHz vs. 4MHz what the spec says. Eg.:

/*
 * Program the gmbus_freq based on the cdclk frequency.
 * BSpec erroneously claims we should aim for 4MHz, but
 * in fact 1MHz is the correct frequency.
 */

Maybe Daniel can add that when applying the patch...

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	if (WARN_ON(gmbus_freq == 0))
> +		return;
> +
> +	I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
> +}
> +
>  void
>  intel_i2c_reset(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/*
> +	 * In BIOS-less system, program the correct gmbus frequency
> +	 * before reading edid.
> +	 */
> +	if (IS_VALLEYVIEW(dev))
> +		gmbus_set_freq(dev_priv);
> +
>  	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
>  	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
>  }
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Sept. 27, 2013, 7:31 p.m. UTC | #2
On Fri, Sep 27, 2013 at 01:07:19PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 27, 2013 at 03:31:00PM +0800, Chon Ming Lee wrote:
> > CDCLK is used to generate the gmbus clock.  This is normally done by
> > BIOS. Program the value if the BIOS-less system doesn't do it.
> > 
> > v2: Move this to intel_i2c_reset to allow reprogram the gmbus frequency
> > during resume. (Daniel)
> > 
> > v3: Change GMBUS_FREQ to GMBUSFREQ_VLV, and use VLV_DISPLAY_BASE.
> > (Ville).
> > 	Remove cdclk_ratio[] table, and calculate the cdclk ratio instead.
> > (Ville).
> >  	Change the shift then mask for reg read, to mask first, then shift.
> > (Ville).
> > 	Remove the gmbus frequency calculation = cdclk/1.01.  Based on BIOS
> > programming, gmbus frequency = cdclk frequency. (Ville)
> > 	Add get_disp_clk_div, which can use to get cdclk/czclk divide.
> > 
> > v4: Fix the mmio_offset base for CZCLK_CDCLK_FREQ_RATIO, gmbus_freq
> > calculation, and duplicate check for gmbus_freq. (Ville)
> > 
> > In VLV, the spec is wrong about 4Mhz reference frequency for GMBUS. It
> > should be 1Mhz.
> > 
> > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |    8 +++++
> >  drivers/gpu/drm/i915/intel_i2c.c |   57 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a0a9811..b37dfd8 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -391,6 +391,8 @@
> >  #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
> >  
> >  /* vlv2 north clock has */
> > +#define CCK_FUSE_REG				0x8
> > +#define  CCK_FUSE_HPLL_FREQ_MASK		0x3
> >  #define CCK_REG_DSI_PLL_FUSE			0x44
> >  #define CCK_REG_DSI_PLL_CONTROL			0x48
> >  #define  DSI_PLL_VCO_EN				(1 << 31)
> > @@ -1438,6 +1440,12 @@
> >  
> >  #define MI_ARB_VLV		(VLV_DISPLAY_BASE + 0x6504)
> >  
> > +#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
> > +#define   CDCLK_FREQ_SHIFT	4
> > +#define   CDCLK_FREQ_MASK	(0x1f << CDCLK_FREQ_SHIFT)
> > +#define   CZCLK_FREQ_MASK	0xf
> > +#define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
> > +
> >  /*
> >   * Palette regs
> >   */
> > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> > index d1c1e0f7..b579ffd 100644
> > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > @@ -34,6 +34,11 @@
> >  #include <drm/i915_drm.h>
> >  #include "i915_drv.h"
> >  
> > +enum disp_clk {
> > +	CDCLK,
> > +	CZCLK
> > +};
> > +
> >  struct gmbus_port {
> >  	const char *name;
> >  	int reg;
> > @@ -58,10 +63,62 @@ to_intel_gmbus(struct i2c_adapter *i2c)
> >  	return container_of(i2c, struct intel_gmbus, adapter);
> >  }
> >  
> > +static int get_disp_clk_div(struct drm_i915_private *dev_priv, enum disp_clk clk)
> > +{
> > +	u32 reg_val;
> > +	int clk_ratio;
> > +
> > +	reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
> > +
> > +	if (clk == CDCLK)
> > +		clk_ratio = ((reg_val & CDCLK_FREQ_MASK) >> CDCLK_FREQ_SHIFT) + 1;
> > +	else
> > +		clk_ratio = (reg_val & CZCLK_FREQ_MASK) + 1;
> > +
> > +	return clk_ratio;
> > +}
> > +
> > +static void gmbus_set_freq(struct drm_i915_private *dev_priv)
> > +{
> > +	int vco_freq[] = { 800, 1600, 2000, 2400 };
> > +	int gmbus_freq = 0, cdclk_div, hpll_freq;
> > +
> > +	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
> > +
> > +	/* Skip setting the gmbus freq if BIOS has already programmed it */
> > +	if (I915_READ(GMBUSFREQ_VLV) != 0xA0)
> > +		return;
> > +
> > +	/* Obtain SKU information */
> > +	mutex_lock(&dev_priv->dpio_lock);
> > +	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) & CCK_FUSE_HPLL_FREQ_MASK;
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> > +	/* Get the CDCLK divide ratio */
> > +	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
> > +
> > +	/* Program the gmbus_freq based on the cdclk frequency */
> > +	if (cdclk_div)
> > +		gmbus_freq = (vco_freq[hpll_freq] << 1) / cdclk_div;
> 
> 
> OK based on the information that you dug up that we do need to aim for
> 1MHz GMBUS freq, the patch looks good to me. I think we should add a
> comment here about the 1MHz vs. 4MHz what the spec says. Eg.:
> 
> /*
>  * Program the gmbus_freq based on the cdclk frequency.
>  * BSpec erroneously claims we should aim for 4MHz, but
>  * in fact 1MHz is the correct frequency.
>  */
> 
> Maybe Daniel can add that when applying the patch...

Done. Also I've massaged the patch a bit to appease checkpatch. Please run
patches through that tool before submitting ...

> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a0a9811..b37dfd8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -391,6 +391,8 @@ 
 #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
 
 /* vlv2 north clock has */
+#define CCK_FUSE_REG				0x8
+#define  CCK_FUSE_HPLL_FREQ_MASK		0x3
 #define CCK_REG_DSI_PLL_FUSE			0x44
 #define CCK_REG_DSI_PLL_CONTROL			0x48
 #define  DSI_PLL_VCO_EN				(1 << 31)
@@ -1438,6 +1440,12 @@ 
 
 #define MI_ARB_VLV		(VLV_DISPLAY_BASE + 0x6504)
 
+#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
+#define   CDCLK_FREQ_SHIFT	4
+#define   CDCLK_FREQ_MASK	(0x1f << CDCLK_FREQ_SHIFT)
+#define   CZCLK_FREQ_MASK	0xf
+#define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
+
 /*
  * Palette regs
  */
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index d1c1e0f7..b579ffd 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -34,6 +34,11 @@ 
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
+enum disp_clk {
+	CDCLK,
+	CZCLK
+};
+
 struct gmbus_port {
 	const char *name;
 	int reg;
@@ -58,10 +63,62 @@  to_intel_gmbus(struct i2c_adapter *i2c)
 	return container_of(i2c, struct intel_gmbus, adapter);
 }
 
+static int get_disp_clk_div(struct drm_i915_private *dev_priv, enum disp_clk clk)
+{
+	u32 reg_val;
+	int clk_ratio;
+
+	reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
+
+	if (clk == CDCLK)
+		clk_ratio = ((reg_val & CDCLK_FREQ_MASK) >> CDCLK_FREQ_SHIFT) + 1;
+	else
+		clk_ratio = (reg_val & CZCLK_FREQ_MASK) + 1;
+
+	return clk_ratio;
+}
+
+static void gmbus_set_freq(struct drm_i915_private *dev_priv)
+{
+	int vco_freq[] = { 800, 1600, 2000, 2400 };
+	int gmbus_freq = 0, cdclk_div, hpll_freq;
+
+	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
+
+	/* Skip setting the gmbus freq if BIOS has already programmed it */
+	if (I915_READ(GMBUSFREQ_VLV) != 0xA0)
+		return;
+
+	/* Obtain SKU information */
+	mutex_lock(&dev_priv->dpio_lock);
+	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) & CCK_FUSE_HPLL_FREQ_MASK;
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	/* Get the CDCLK divide ratio */
+	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
+
+	/* Program the gmbus_freq based on the cdclk frequency */
+	if (cdclk_div)
+		gmbus_freq = (vco_freq[hpll_freq] << 1) / cdclk_div;
+
+	if (WARN_ON(gmbus_freq == 0))
+		return;
+
+	I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
+}
+
 void
 intel_i2c_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/*
+	 * In BIOS-less system, program the correct gmbus frequency
+	 * before reading edid.
+	 */
+	if (IS_VALLEYVIEW(dev))
+		gmbus_set_freq(dev_priv);
+
 	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
 	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
 }