On Fri, Sep 13, 2013 at 5:59 AM, Chon Ming Lee <chon.ming.lee@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx