On Mon, Apr 28, 2014 at 05:54:24PM +0300, Imre Deak wrote: > On Wed, 2014-04-09 at 13:28 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Chon Ming Lee <chon.ming.lee@xxxxxxxxx> > > > > During cold boot, the display controller needs to deassert the common > > lane reset. Only do it once during intel_init_dpio for both PHYx2 and > > PHYx1. > > > > Besides, assert the common lane reset when disable pll. This still > > to be determined whether need to do it by driver. > > > > Signed-off-by: Chon Ming Lee <chon.ming.lee@xxxxxxxxx> > > [vsyrjala: Don't disable DPIO PLL when using DSI] > > [vsyrjala: Don't call vlv_disable_pll() by accident on CHV] > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 8 +++++ > > drivers/gpu/drm/i915/intel_display.c | 66 ++++++++++++++++++++++++++++-------- > > 2 files changed, 59 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 8aea092..8fcf4ea 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -1391,6 +1391,14 @@ enum punit_power_well { > > /* Additional CHV pll/phy registers */ > > #define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240) > > #define DPLL_PORTD_READY_MASK (0xf) > > +#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100) > > +#define PHY_COM_LANE_RESET_DEASSERT(phy, val) \ > > + ((phy == DPIO_PHY0) ? (val | 1) : (val | 2)) > > +#define PHY_COM_LANE_RESET_ASSERT(phy, val) \ > > + ((phy == DPIO_PHY0) ? (val & ~1) : (val & ~2)) > > +#define DISPLAY_PHY_STATUS (VLV_DISPLAY_BASE + 0x60104) > > +#define PHY_POWERGOOD(phy) ((phy == DPIO_PHY0) ? (1<<31) : (1<<30)) > > + > > /* > > * The i830 generation, in LVDS mode, defines P1 as the bit number set within > > * this field (only one bit may be set). > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 153f244..e33667d 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1395,17 +1395,36 @@ static void intel_reset_dpio(struct drm_device *dev) > > DPLL_REFA_CLK_ENABLE_VLV | > > DPLL_INTEGRATED_CRI_CLK_VLV); > > > > - /* > > - * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx - > > - * 6. De-assert cmn_reset/side_reset. Same as VLV X0. > > - * a. GUnit 0x2110 bit[0] set to 1 (def 0) > > - * b. The other bits such as sfr settings / modesel may all be set > > - * to 0. > > This is VLV specific, so ok to be moved, > > > - * > > - * This should only be done on init and resume from S3 with both > > - * PLLs disabled, or we risk losing DPIO and PLL synchronization. > > - */ > > but this is also true for CHV, so should stay. I've copypasted this block to the chv comment while merging. -Daniel > > > - I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST); > > + if (IS_CHERRYVIEW(dev)) { > > + enum dpio_phy phy; > > + u32 val; > > + > > + for (phy = DPIO_PHY0; phy < I915_NUM_PHYS_VLV; phy++) { > > + /* Poll for phypwrgood signal */ > > + if (wait_for(I915_READ(DISPLAY_PHY_STATUS) & > > + PHY_POWERGOOD(phy), 1)) > > + DRM_ERROR("Display PHY %d is not power up\n", phy); > > + > > + /* Deassert common lane reset for PHY*/ > > + val = I915_READ(DISPLAY_PHY_CONTROL); > > + I915_WRITE(DISPLAY_PHY_CONTROL, > > + PHY_COM_LANE_RESET_DEASSERT(phy, val)); > > Would be clearer not to hide the 'or' in the macro and let > PHY_COM_LANE_RESET_DEASSERT be just the flag itself and do here > I915_WRITE(DISPLAY_PHY_CONTROL, val | PHY_COM_LANE_RESET_DEASSERT(phy)); > > The above issues are minor, so even without fixing them this patch is > Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> > > > + } > > + > > + } else { > > + /* > > + * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx - > > + * 6. De-assert cmn_reset/side_reset. Same as VLV X0. > > + * a. GUnit 0x2110 bit[0] set to 1 (def 0) > > + * b. The other bits such as sfr settings / modesel may all > > + * be set to 0. > > + * > > + * This should only be done on init and resume from S3 with > > + * both PLLs disabled, or we risk losing DPIO and PLL > > + * synchronization. > > + */ > > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST); > > + } > > } > > > > static void vlv_enable_pll(struct intel_crtc *crtc) > > @@ -1529,6 +1548,19 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) > > val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV; > > I915_WRITE(DPLL(pipe), val); > > POSTING_READ(DPLL(pipe)); > > + > > +} > > + > > +static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) > > +{ > > + int dpll = DPLL(pipe); > > + u32 val; > > + > > + /* Set PLL en = 0 */ > > + val = I915_READ(dpll); > > + val &= ~DPLL_VCO_ENABLE; > > + I915_WRITE(dpll, val); > > + > > } > > > > void vlv_wait_port_ready(struct drm_i915_private *dev_priv, > > @@ -4511,10 +4543,14 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) > > if (encoder->post_disable) > > encoder->post_disable(encoder); > > > > - if (IS_VALLEYVIEW(dev) && !intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI)) > > - vlv_disable_pll(dev_priv, pipe); > > - else if (!IS_VALLEYVIEW(dev)) > > - i9xx_disable_pll(dev_priv, pipe); > > + if (!intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI)) { > > + if (IS_CHERRYVIEW(dev)) > > + chv_disable_pll(dev_priv, pipe); > > + else if (IS_VALLEYVIEW(dev)) > > + vlv_disable_pll(dev_priv, pipe); > > + else > > + i9xx_disable_pll(dev_priv, pipe); > > + } > > > > intel_crtc->active = false; > > intel_update_watermarks(crtc); > > _______________________________________________ > 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