| Message ID | 1379044785-13193-1-git-send-email-chon.ming.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 13, 2013 at 5:59 AM, Chon Ming Lee <chon.ming.lee@intel.com> wrote: > In non PC system, such as IVI, may not use BIOS, instead it uses boot > loader with only minimal system initialization. Most of the time, boot > loader doesn't come with VBIOS, and depends on device driver to fully > initialize the display controller and GPU. > > For Valleyview, without VBIOS, i915 fails to work. The patch add some > missing init code, such as doing a DPIO CMNRESET and program the GMBUS > frequency. > > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> Generally I prefer patches with tighter topics, grouped into a series. For this one I'd split it up into the gmbus setup and the dpio reset, so 2 patches. > --- > drivers/gpu/drm/i915/i915_reg.h | 8 +++++ > drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++++++++++ > 2 files changed, 59 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 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) > @@ -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_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3c0e0cf..9ef1d28 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9497,6 +9497,31 @@ static bool has_edp_a(struct drm_device *dev) > return true; > } > > +static void gmbus_set_freq(struct drm_i915_private *dev_priv, u32 hpll_freq) > +{ > + 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; > + u32 reg_val; > + > + BUG_ON(hpll_freq > ARRAY_SIZE(vco_freq)); > + > + reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO); > + > + cdclk = ((reg_val >> CDCLK_FREQ_SHIFT) & CDCLK_FREQ_MASK) - 1; > + > + if (cdclk_ratio[cdclk]) > + gmbus_freq = vco_freq[hpll_freq] / cdclk_ratio[cdclk] * 10; > + > + WARN_ON(gmbus_freq == 0); > + > + if (gmbus_freq != 0) > + I915_WRITE(GMBUS_FREQ, gmbus_freq); > + > +} > + > static void intel_setup_outputs(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -9555,6 +9580,32 @@ static void intel_setup_outputs(struct drm_device *dev) Doing this in setup_outputs feels like hiding stuff ;-) I know that there's already the pch refclock setup here, but I've just submitted a patch to get it out of there. The problem is that setup_outputs isn't called on resume, and I suspect we actually need this. The dpio reset is probably best stuffed into intel_uncore_sanitize > if (I915_READ(PCH_DP_D) & DP_DETECTED) > intel_dp_init(dev, PCH_DP_D, PORT_D); > } else if (IS_VALLEYVIEW(dev)) { > + u32 reg_val; > + > + /* Trigger DPIO CMN RESET, require especially in BIOS less > + * system > + */ > + reg_val = I915_READ(DPIO_CTL); > + if (!(reg_val & 0x1)) { > + I915_WRITE(DPIO_CTL, 0x0); > + I915_WRITE(DPIO_CTL, 0x1); > + POSTING_READ(DPIO_CTL); > + } Atm we don't have a suitable "early platform setup" function which is called from the right places. I'd shove this into intel_uncore.c: intel_uncore_sanitize for now, that should work. Maybe once your patches are in I'll throw a intel_ealy_init_hw refactoring on top to avoid sprinkling things all over the place between the driver load and the resume codepaths. > + > + /* In BIOS-less system, program the correct gmbus frequency > + * before reading edid. > + */ > + > + /* 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); I'd move this into the gmbus_freq setup function. > + > + reg_val &= CCK_FUSE_HPLL_FREQ_MASK; > + > + /* Write CDCLK to GMBUS freq for GMBUS clk generation. */ > + gmbus_set_freq(dev_priv, reg_val); Imo it's better to move this to intel_i2c.c: intel_i2c_reset. That function is already called from all the right places. Cheers, Daniel > + > /* Check for built-in panel first. Shares lanes with HDMI on SDVOC */ > if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & SDVO_DETECTED) { > intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC, > -- > 1.7.7.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 09/12 18:30, Daniel Vetter wrote: > On Fri, Sep 13, 2013 at 5:59 AM, Chon Ming Lee <chon.ming.lee@intel.com> wrote: > > In non PC system, such as IVI, may not use BIOS, instead it uses boot > > loader with only minimal system initialization. Most of the time, boot > > loader doesn't come with VBIOS, and depends on device driver to fully > > initialize the display controller and GPU. > > > > For Valleyview, without VBIOS, i915 fails to work. The patch add some > > missing init code, such as doing a DPIO CMNRESET and program the GMBUS > > frequency. > > > > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> > > Generally I prefer patches with tighter topics, grouped into a series. > For this one I'd split it up into the gmbus setup and the dpio reset, > so 2 patches. > > > --- > > drivers/gpu/drm/i915/i915_reg.h | 8 +++++ > > drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++++++++++ > > 2 files changed, 59 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 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) > > @@ -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_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3c0e0cf..9ef1d28 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9497,6 +9497,31 @@ static bool has_edp_a(struct drm_device *dev) > > return true; > > } > > > > +static void gmbus_set_freq(struct drm_i915_private *dev_priv, u32 hpll_freq) > > +{ > > + 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; > > + u32 reg_val; > > + > > + BUG_ON(hpll_freq > ARRAY_SIZE(vco_freq)); > > + > > + reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO); > > + > > + cdclk = ((reg_val >> CDCLK_FREQ_SHIFT) & CDCLK_FREQ_MASK) - 1; > > + > > + if (cdclk_ratio[cdclk]) > > + gmbus_freq = vco_freq[hpll_freq] / cdclk_ratio[cdclk] * 10; > > + > > + WARN_ON(gmbus_freq == 0); > > + > > + if (gmbus_freq != 0) > > + I915_WRITE(GMBUS_FREQ, gmbus_freq); > > + > > +} > > + > > static void intel_setup_outputs(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -9555,6 +9580,32 @@ static void intel_setup_outputs(struct drm_device *dev) > > Doing this in setup_outputs feels like hiding stuff ;-) I know that > there's already the pch refclock setup here, but I've just submitted a > patch to get it out of there. > > The problem is that setup_outputs isn't called on resume, and I > suspect we actually need this. > > The dpio reset is probably best stuffed into intel_uncore_sanitize > > if (I915_READ(PCH_DP_D) & DP_DETECTED) > > intel_dp_init(dev, PCH_DP_D, PORT_D); > > } else if (IS_VALLEYVIEW(dev)) { > > + u32 reg_val; > > + > > + /* Trigger DPIO CMN RESET, require especially in BIOS less > > + * system > > + */ > > + reg_val = I915_READ(DPIO_CTL); > > + if (!(reg_val & 0x1)) { > > + I915_WRITE(DPIO_CTL, 0x0); > > + I915_WRITE(DPIO_CTL, 0x1); > > + POSTING_READ(DPIO_CTL); > > + } > > Atm we don't have a suitable "early platform setup" function which is > called from the right places. I'd shove this into intel_uncore.c: > intel_uncore_sanitize for now, that should work. > > Maybe once your patches are in I'll throw a intel_ealy_init_hw > refactoring on top to avoid sprinkling things all over the place > between the driver load and the resume codepaths. > I have moved according what you suggest. Now, the system resume is able to work now. Will send out the revise patch. > > + > > + /* In BIOS-less system, program the correct gmbus frequency > > + * before reading edid. > > + */ > > + > > + /* 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); > > I'd move this into the gmbus_freq setup function. > > > + > > + reg_val &= CCK_FUSE_HPLL_FREQ_MASK; > > + > > + /* Write CDCLK to GMBUS freq for GMBUS clk generation. */ > > + gmbus_set_freq(dev_priv, reg_val); > > Imo it's better to move this to intel_i2c.c: intel_i2c_reset. That > function is already called from all the right places. > > Cheers, Daniel > > > + > > /* Check for built-in panel first. Shares lanes with HDMI on SDVOC */ > > if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & SDVO_DETECTED) { > > intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC, > > -- > > 1.7.7.6 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
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 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) @@ -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_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c0e0cf..9ef1d28 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9497,6 +9497,31 @@ static bool has_edp_a(struct drm_device *dev) return true; } +static void gmbus_set_freq(struct drm_i915_private *dev_priv, u32 hpll_freq) +{ + 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; + u32 reg_val; + + BUG_ON(hpll_freq > ARRAY_SIZE(vco_freq)); + + reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO); + + cdclk = ((reg_val >> CDCLK_FREQ_SHIFT) & CDCLK_FREQ_MASK) - 1; + + if (cdclk_ratio[cdclk]) + gmbus_freq = vco_freq[hpll_freq] / cdclk_ratio[cdclk] * 10; + + WARN_ON(gmbus_freq == 0); + + if (gmbus_freq != 0) + I915_WRITE(GMBUS_FREQ, gmbus_freq); + +} + static void intel_setup_outputs(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -9555,6 +9580,32 @@ static void intel_setup_outputs(struct drm_device *dev) if (I915_READ(PCH_DP_D) & DP_DETECTED) intel_dp_init(dev, PCH_DP_D, PORT_D); } else if (IS_VALLEYVIEW(dev)) { + u32 reg_val; + + /* Trigger DPIO CMN RESET, require especially in BIOS less + * system + */ + reg_val = I915_READ(DPIO_CTL); + if (!(reg_val & 0x1)) { + I915_WRITE(DPIO_CTL, 0x0); + I915_WRITE(DPIO_CTL, 0x1); + POSTING_READ(DPIO_CTL); + } + + /* In BIOS-less system, program the correct gmbus frequency + * before reading edid. + */ + + /* 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); + + reg_val &= CCK_FUSE_HPLL_FREQ_MASK; + + /* Write CDCLK to GMBUS freq for GMBUS clk generation. */ + gmbus_set_freq(dev_priv, reg_val); + /* Check for built-in panel first. Shares lanes with HDMI on SDVOC */ if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & SDVO_DETECTED) { intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
In non PC system, such as IVI, may not use BIOS, instead it uses boot loader with only minimal system initialization. Most of the time, boot loader doesn't come with VBIOS, and depends on device driver to fully initialize the display controller and GPU. For Valleyview, without VBIOS, i915 fails to work. The patch add some missing init code, such as doing a DPIO CMNRESET and program the GMBUS frequency. Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 8 +++++ drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 0 deletions(-)