Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
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
> > ---
> > 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_MASK0xf800
> >
> > /* 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_CONTROL0x48
> > #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
> > #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ä
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
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
> ---
> 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 0xf800
>
> /* 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_MASK0xf
> +#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
> #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ä
> +
> + 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
[Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
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
---
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_MASK0xf800
/* 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_CONTROL0x48
#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
#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);
}
--
1.7.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
On Tue, Sep 24, 2013 at 09:18:57PM +0800, Lee, Chon Ming wrote:
> On 09/24 15:54, Ville Syrjälä wrote:
> > On Tue, Sep 24, 2013 at 05:47:57PM +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.
> > >
> > > Signed-off-by: Chon Ming Lee
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h |8 +
> > > drivers/gpu/drm/i915/intel_i2c.c | 60
> > > ++
> > > 2 files changed, 68 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index c02f893..e8315c6 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 0xf800
> > >
> > > /* 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 (dev_priv->info->display_mmio_offset +
> > > 0x6508)
> > ^^^
> >
> > This too could be VLV_DISPLAY_BASE.
>
> ah, thanks for catching this.
> >
> > > +#define CDCLK_FREQ_SHIFT 4
> > > +#define CDCLK_FREQ_MASK(0x1f << CDCLK_FREQ_SHIFT)
> > > +#define CZCLK_FREQ_MASK0xf
> > > +#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..b225d60 100644
> > > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > > @@ -34,6 +34,11 @@
> > > #include
> > > #include "i915_drv.h"
> > >
> > > +enum disp_clk {
> > > + CDCLK = 0,
> >
> >
> > That part is not necessary. Enums start at 0 by default.
> >
> > > + CZCLK
> > > +};
> > > +
> > > struct gmbus_port {
> > > const char *name;
> > > int reg;
> > > @@ -58,10 +63,65 @@ 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 * 5;
> >
> > This factor 5 multiplication is a bit pointless.
> >
> This is to get the divide ratio. I don't understand why you think it is not
> needed.
We can leave it as .1 binary fixed point. The *5 just convertes it
into .1 decimal fixed point. We don't need the extra precision.
Then you just replace the '* 10' in the gmbus_freq calculation with
'<< 1'.
>
>
> > > +}
> > > +
> > > +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;
> > > + u32 reg_val;
> > > +
> > > + 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 to determine the correct CDCLK */
> > > + mutex_lock(&dev_priv->dpio_lock);
> > > + reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
> > > + mutex_unlock(&dev_priv->dpio_lock);
> > > +
> > > + hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
> > > +
> > > + /* 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] * 10 / cdclk_div;
> >
> > Should be '... / (4 * cdclk_div)' to get
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
On 09/24 15:54, Ville Syrjälä wrote:
> On Tue, Sep 24, 2013 at 05:47:57PM +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.
> >
> > Signed-off-by: Chon Ming Lee
> > ---
> > drivers/gpu/drm/i915/i915_reg.h |8 +
> > drivers/gpu/drm/i915/intel_i2c.c | 60
> > ++
> > 2 files changed, 68 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index c02f893..e8315c6 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_MASK0xf800
> >
> > /* 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_CONTROL0x48
> > #define DSI_PLL_VCO_EN(1 << 31)
> > @@ -1438,6 +1440,12 @@
> >
> > #define MI_ARB_VLV (VLV_DISPLAY_BASE + 0x6504)
> >
> > +#define CZCLK_CDCLK_FREQ_RATIO (dev_priv->info->display_mmio_offset +
> > 0x6508)
> ^^^
>
> This too could be VLV_DISPLAY_BASE.
ah, thanks for catching this.
>
> > +#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..b225d60 100644
> > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > @@ -34,6 +34,11 @@
> > #include
> > #include "i915_drv.h"
> >
> > +enum disp_clk {
> > + CDCLK = 0,
>
>
> That part is not necessary. Enums start at 0 by default.
>
> > + CZCLK
> > +};
> > +
> > struct gmbus_port {
> > const char *name;
> > int reg;
> > @@ -58,10 +63,65 @@ 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 * 5;
>
> This factor 5 multiplication is a bit pointless.
>
This is to get the divide ratio. I don't understand why you think it is not
needed.
> > +}
> > +
> > +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;
> > + u32 reg_val;
> > +
> > + 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 to determine the correct CDCLK */
> > + mutex_lock(&dev_priv->dpio_lock);
> > + reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
> > + mutex_unlock(&dev_priv->dpio_lock);
> > +
> > + hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
> > +
> > + /* 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] * 10 / cdclk_div;
>
> Should be '... / (4 * cdclk_div)' to get the 4MHz.
>
Actually, the calculation should be correct. I am getting the same result when
compare to BIOS. The spec is quite confuse when mention about 4MHz.
> > +
> > + WARN_ON(gmbus_freq == 0);
> > +
> > + if (gmbus_freq != 0)
> > + I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
>
> We could avoid the double comparison like so:
>
> if (WARN_ON(gmbus_freq == 0))
> return;
>
> I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
>
Noted on this.
> > +
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
On Tue, Sep 24, 2013 at 05:47:57PM +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.
>
> Signed-off-by: Chon Ming Lee
> ---
> drivers/gpu/drm/i915/i915_reg.h |8 +
> drivers/gpu/drm/i915/intel_i2c.c | 60
> ++
> 2 files changed, 68 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c02f893..e8315c6 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 0xf800
>
> /* 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 (dev_priv->info->display_mmio_offset +
> 0x6508)
^^^
This too could be VLV_DISPLAY_BASE.
> +#define CDCLK_FREQ_SHIFT 4
> +#define CDCLK_FREQ_MASK(0x1f << CDCLK_FREQ_SHIFT)
> +#define CZCLK_FREQ_MASK0xf
> +#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..b225d60 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -34,6 +34,11 @@
> #include
> #include "i915_drv.h"
>
> +enum disp_clk {
> + CDCLK = 0,
That part is not necessary. Enums start at 0 by default.
> + CZCLK
> +};
> +
> struct gmbus_port {
> const char *name;
> int reg;
> @@ -58,10 +63,65 @@ 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 * 5;
This factor 5 multiplication is a bit pointless.
> +}
> +
> +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;
> + u32 reg_val;
> +
> + 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 to determine the correct CDCLK */
> + mutex_lock(&dev_priv->dpio_lock);
> + reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
> +
> + /* 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] * 10 / cdclk_div;
Should be '... / (4 * cdclk_div)' to get the 4MHz.
> +
> + WARN_ON(gmbus_freq == 0);
> +
> + if (gmbus_freq != 0)
> + I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
We could avoid the double comparison like so:
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.
> + */
The style police may complain about this multiline comment. The correct
format is:
/*
* foo
* bar
*/
> + 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 + G
[Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
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.
Signed-off-by: Chon Ming Lee
---
drivers/gpu/drm/i915/i915_reg.h |8 +
drivers/gpu/drm/i915/intel_i2c.c | 60 ++
2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c02f893..e8315c6 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_MASK0xf800
/* 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_CONTROL0x48
#define DSI_PLL_VCO_EN(1 << 31)
@@ -1438,6 +1440,12 @@
#define MI_ARB_VLV (VLV_DISPLAY_BASE + 0x6504)
+#define CZCLK_CDCLK_FREQ_RATIO (dev_priv->info->display_mmio_offset + 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..b225d60 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -34,6 +34,11 @@
#include
#include "i915_drv.h"
+enum disp_clk {
+ CDCLK = 0,
+ CZCLK
+};
+
struct gmbus_port {
const char *name;
int reg;
@@ -58,10 +63,65 @@ 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 * 5;
+}
+
+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;
+ u32 reg_val;
+
+ 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 to determine the correct CDCLK */
+ mutex_lock(&dev_priv->dpio_lock);
+ reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+ mutex_unlock(&dev_priv->dpio_lock);
+
+ hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
+
+ /* 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] * 10 / cdclk_div;
+
+ WARN_ON(gmbus_freq == 0);
+
+ if (gmbus_freq != 0)
+ 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
[Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK for VLV.
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.
Signed-off-by: Chon Ming Lee
---
drivers/gpu/drm/i915/i915_reg.h |8 +
drivers/gpu/drm/i915/intel_i2c.c | 60 ++
2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c02f893..e8315c6 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_MASK0xf800
/* 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_CONTROL0x48
#define DSI_PLL_VCO_EN(1 << 31)
@@ -1438,6 +1440,12 @@
#define MI_ARB_VLV (VLV_DISPLAY_BASE + 0x6504)
+#define CZCLK_CDCLK_FREQ_RATIO (dev_priv->info->display_mmio_offset + 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..b225d60 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -34,6 +34,11 @@
#include
#include "i915_drv.h"
+enum disp_clk {
+ CDCLK = 0,
+ CZCLK
+};
+
struct gmbus_port {
const char *name;
int reg;
@@ -58,10 +63,65 @@ 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 * 5;
+}
+
+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;
+ u32 reg_val;
+
+ 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 to determine the correct CDCLK */
+ mutex_lock(&dev_priv->dpio_lock);
+ reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+ mutex_unlock(&dev_priv->dpio_lock);
+
+ hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
+
+ /* 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] * 10 / cdclk_div;
+
+ WARN_ON(gmbus_freq == 0);
+
+ if (gmbus_freq != 0)
+ 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
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK
On Fri, Sep 13, 2013 at 1:23 PM, Ville Syrjälä wrote: > > So you might want to turn this function into some generic function to > calculate both GMBUS and AUX clocks for VLV. You could just take the > target frequency (2 or 4 in these cases) as a parameter and return the > calculated divider. I think that should be done in the patch series which enables dynamic cdclk changes (if we get there). Then we can also think about what we need to do with the dp aux stuff, especially since we already have quite a bit of logic in there to deal with different dp ref clocks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK
On Fri, Sep 13, 2013 at 02:39:21PM +0800, Chon Ming Lee wrote:
> CDCLK is used to generate the gmbus clock. This is normally done by
> BIOS. This is only for valleyview platform.
>
> v2: Move this to intel_i2c_reset to allow reprogram the gmbus frequency
> during resume. (Daniel)
>
> Signed-off-by: Chon Ming Lee
> ---
> drivers/gpu/drm/i915/i915_reg.h |8 +++
> drivers/gpu/drm/i915/intel_i2c.c | 43
> ++
> 2 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bcee89b..8ddf58a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -382,6 +382,8 @@
> #define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800
>
> /* 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)
> @@ -1424,6 +1426,12 @@
>
> #define MI_ARB_VLV (VLV_DISPLAY_BASE + 0x6504)
>
> +#define CZCLK_CDCLK_FREQ_RATIO (dev_priv->info->display_mmio_offset +
> 0x6508)
> +#define CDCLK_FREQ_SHIFT 4
> +#define CDCLK_FREQ_MASK0x1f
> +#define CZCLK_FREQ_MASK0xf
> +#define GMBUS_FREQ (dev_priv->info->display_mmio_offset + 0x6510)
I believe both of these registers are VLV specific, so you can use
(VLV_DISPLAY_BASE + 0x..)
There's a GMBUSFREQ on CTG at least, but it's a PCI config register
at offset 0xcc. But still, probably best to call the VLV register
GMBUSFREQ_VLV.
> +
> /*
> * Palette regs
> */
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c
> b/drivers/gpu/drm/i915/intel_i2c.c
> index d1c1e0f7..a8c4165 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -58,10 +58,53 @@ to_intel_gmbus(struct i2c_adapter *i2c)
> return container_of(i2c, struct intel_gmbus, adapter);
> }
>
> +static void gmbus_set_freq(struct drm_i915_private *dev_priv)
> +{
> + int cdclk_ratio[] = { 10, 15, 20, 25, 30, 0, 40, 45, 50, 0,
> + 60, 0, 0, 75, 80, 0, 90, 0, 100, 0,
> + 0, 0, 120, 0, 0, 0, 0, 0, 150, 0, 160 };
We can avoid the table. That's just (10 + i * 5), or could be just (2 + i)
and use 2 as the divider.
> + int vco_freq[] = { 800, 1600, 2000, 2400 };
> + int gmbus_freq = 0, cdclk, hpll_freq;
> + u32 reg_val;
> +
> + BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
> +
> + /* Obtain SKU information to determine the correct CDCLK */
> + mutex_lock(&dev_priv->dpio_lock);
> + reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
> +
> + /* Get the CDCLK frequency */
> + reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
> +
> + cdclk = ((reg_val >> CDCLK_FREQ_SHIFT) & CDCLK_FREQ_MASK) - 1;
I think we tend to define the masks to be shifted. So mask first, then
shift.
And as stated, we don't need the table. So just make it:
/* in .1 fixed point */
cdclk_div = ((reg_val & MASK) >> SHIFT) + 1;
> +
> + /* To enable hotplug detect, the gmbus frequency need to set as
> + * cdclk/1.01
> + */
> + if (cdclk_ratio[cdclk])
> + gmbus_freq = vco_freq[hpll_freq] / cdclk_ratio[cdclk] * 101 /
> 10;
Where does that 1.01 come from? For 200MHz CDCLK GMBUS would be running
at 198MHz? Also I think what you might be actually achieving here is is
a GMBUS frequency of 1/1.01 MHz, since the register supposedly takes the
number of CDCLKs per GMBUS clock.
The spec s a big poorly worded, but I think what it's saying is that
we need to calculate the number of CDCLKs per a 4MHz GMBUS reference
clock. So something like should do the right thing:
/* how many CDCLKs for 4MHz GMBUS reference frequency */
gmbus_freq = ((vco_freq << 1) / (cdclk_div * 4);
Not sure if we should round up or down.
Obviosuly we need to recalculate this as needed when we start to change
CDCLK dynamically. Same for the AUX clock. Actually we should be
able to use the same function to calculate both. The AUX clock is just
2MHz instead of 4MHz.
So you might want to turn this function into some generic function to
calculate both GMBUS and AUX clocks for VLV. You could just take the
target frequency (2 or 4 in these cases) as a parameter and return the
calculated divider.
> +
> + WARN_ON(gmbus_freq == 0);
> +
> + if (gmbus_freq != 0)
> + I915_WRITE(GMBUS_FREQ, 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_p
[Intel-gfx] [PATCH 2/2] drm/i915: Program GMBUS Frequency based on the CDCLK
CDCLK is used to generate the gmbus clock. This is normally done by
BIOS. This is only for valleyview platform.
v2: Move this to intel_i2c_reset to allow reprogram the gmbus frequency
during resume. (Daniel)
Signed-off-by: Chon Ming Lee
---
drivers/gpu/drm/i915/i915_reg.h |8 +++
drivers/gpu/drm/i915/intel_i2c.c | 43 ++
2 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bcee89b..8ddf58a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -382,6 +382,8 @@
#define FB_FMAX_VMIN_FREQ_LO_MASK0xf800
/* 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_CONTROL0x48
#define DSI_PLL_VCO_EN(1 << 31)
@@ -1424,6 +1426,12 @@
#define MI_ARB_VLV (VLV_DISPLAY_BASE + 0x6504)
+#define CZCLK_CDCLK_FREQ_RATIO (dev_priv->info->display_mmio_offset + 0x6508)
+#define CDCLK_FREQ_SHIFT 4
+#define CDCLK_FREQ_MASK 0x1f
+#define CZCLK_FREQ_MASK 0xf
+#define GMBUS_FREQ (dev_priv->info->display_mmio_offset + 0x6510)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index d1c1e0f7..a8c4165 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -58,10 +58,53 @@ to_intel_gmbus(struct i2c_adapter *i2c)
return container_of(i2c, struct intel_gmbus, adapter);
}
+static void gmbus_set_freq(struct drm_i915_private *dev_priv)
+{
+ int cdclk_ratio[] = { 10, 15, 20, 25, 30, 0, 40, 45, 50, 0,
+ 60, 0, 0, 75, 80, 0, 90, 0, 100, 0,
+ 0, 0, 120, 0, 0, 0, 0, 0, 150, 0, 160 };
+ int vco_freq[] = { 800, 1600, 2000, 2400 };
+ int gmbus_freq = 0, cdclk, hpll_freq;
+ u32 reg_val;
+
+ BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
+
+ /* Obtain SKU information to determine the correct CDCLK */
+ mutex_lock(&dev_priv->dpio_lock);
+ reg_val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+ mutex_unlock(&dev_priv->dpio_lock);
+
+ hpll_freq = reg_val & CCK_FUSE_HPLL_FREQ_MASK;
+
+ /* Get the CDCLK frequency */
+ reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
+
+ cdclk = ((reg_val >> CDCLK_FREQ_SHIFT) & CDCLK_FREQ_MASK) - 1;
+
+ /* To enable hotplug detect, the gmbus frequency need to set as
+* cdclk/1.01
+*/
+ if (cdclk_ratio[cdclk])
+ gmbus_freq = vco_freq[hpll_freq] / cdclk_ratio[cdclk] * 101 /
10;
+
+ WARN_ON(gmbus_freq == 0);
+
+ if (gmbus_freq != 0)
+ I915_WRITE(GMBUS_FREQ, 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
