On Mon, Aug 17, 2015 at 09:46:01AM +0530, Deepak wrote: > > > On 07/09/2015 02:15 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Move the CHV clock buffer disable from chv_disable_pll() to the new > > encoder .post_pll_disable() hook. This is more symmetric since the > > clock buffer enable happens from the .pre_pll_enable() hook. > > > > We'll have more use for the new hook soon. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 15 ++++----------- > > drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_hdmi.c | 23 +++++++++++++++++++++++ > > 4 files changed, 51 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3df9cb2..db518a7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1851,17 +1851,6 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) > > val &= ~DPIO_DCLKP_EN; > > vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), val); > > > > - /* disable left/right clock distribution */ > > - if (pipe != PIPE_B) { > > - val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW5_CH0); > > - val &= ~(CHV_BUFLEFTENA1_MASK | CHV_BUFRIGHTENA1_MASK); > > - vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW5_CH0, val); > > - } else { > > - val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW1_CH1); > > - val &= ~(CHV_BUFLEFTENA2_MASK | CHV_BUFRIGHTENA2_MASK); > > - vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW1_CH1, val); > > - } > > - > > mutex_unlock(&dev_priv->sb_lock); > > } > > > > @@ -6171,6 +6160,10 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) > > i9xx_disable_pll(intel_crtc); > > } > > > > + for_each_encoder_on_crtc(dev, crtc, encoder) > > + if (encoder->post_pll_disable) > > + encoder->post_pll_disable(encoder); > > + > We call "vlv_force_pll_off" in vlv_power_sequencer_kick which call > chv_disablepll. > Should we add the "post_pll_disable"in force pll off? Nope. .post_pll_disable() is the counterpart to .pre_pll_enable() which we don't use either in the pps kick procedure. Apparently we don't need to concern ourselves with the clock channel/districtution setp to make the pps kick work. Well, it's either that, or we've just been lucky and those end up being set up in a good way by accident. But now that you mention it, I do start wonder a bit how it really works. So this is probably something I should test more thoroughly at some point. > > if (!IS_GEN2(dev)) > > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); > > } > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 32d7e43..40b8430 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2906,6 +2906,28 @@ static void chv_dp_pre_pll_enable(struct intel_encoder *encoder) > > mutex_unlock(&dev_priv->sb_lock); > > } > > > > +static void chv_dp_post_pll_disable(struct intel_encoder *encoder) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + enum pipe pipe = to_intel_crtc(encoder->base.crtc)->pipe; > > + u32 val; > > + > > + mutex_lock(&dev_priv->sb_lock); > > + > > + /* disable left/right clock distribution */ > > + if (pipe != PIPE_B) { > > + val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW5_CH0); > > + val &= ~(CHV_BUFLEFTENA1_MASK | CHV_BUFRIGHTENA1_MASK); > > + vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW5_CH0, val); > > + } else { > > + val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW1_CH1); > > + val &= ~(CHV_BUFLEFTENA2_MASK | CHV_BUFRIGHTENA2_MASK); > > + vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW1_CH1, val); > > + } > > + > > + mutex_unlock(&dev_priv->sb_lock); > > +} > > + > > /* > > * Native read with retry for link status and receiver capability reads for > > * cases where the sink may still be asleep. > > @@ -5931,6 +5953,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) > > intel_encoder->pre_enable = chv_pre_enable_dp; > > intel_encoder->enable = vlv_enable_dp; > > intel_encoder->post_disable = chv_post_disable_dp; > > + intel_encoder->post_pll_disable = chv_dp_post_pll_disable; > > } else if (IS_VALLEYVIEW(dev)) { > > intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable; > > intel_encoder->pre_enable = vlv_pre_enable_dp; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 4f3b708..42fa135 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -148,6 +148,7 @@ struct intel_encoder { > > void (*mode_set)(struct intel_encoder *intel_encoder); > > void (*disable)(struct intel_encoder *); > > void (*post_disable)(struct intel_encoder *); > > + void (*post_pll_disable)(struct intel_encoder *); > > /* Read out the current hw state of this connector, returning true if > > * the encoder is active. If the encoder is enabled it also set the pipe > > * it is connected to in the pipe parameter. */ > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index 9f79afb..86b1a2c 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -1678,6 +1678,28 @@ static void chv_hdmi_pre_pll_enable(struct intel_encoder *encoder) > > mutex_unlock(&dev_priv->sb_lock); > > } > > > > +static void chv_hdmi_post_pll_disable(struct intel_encoder *encoder) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + enum pipe pipe = to_intel_crtc(encoder->base.crtc)->pipe; > > + u32 val; > > + > > + mutex_lock(&dev_priv->sb_lock); > > + > > + /* disable left/right clock distribution */ > > + if (pipe != PIPE_B) { > > + val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW5_CH0); > > + val &= ~(CHV_BUFLEFTENA1_MASK | CHV_BUFRIGHTENA1_MASK); > > + vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW5_CH0, val); > > + } else { > > + val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW1_CH1); > > + val &= ~(CHV_BUFLEFTENA2_MASK | CHV_BUFRIGHTENA2_MASK); > > + vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW1_CH1, val); > > + } > > + > > + mutex_unlock(&dev_priv->sb_lock); > > +} > > + > > static void vlv_hdmi_post_disable(struct intel_encoder *encoder) > > { > > struct intel_digital_port *dport = enc_to_dig_port(&encoder->base); > > @@ -2073,6 +2095,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port) > > intel_encoder->pre_enable = chv_hdmi_pre_enable; > > intel_encoder->enable = vlv_enable_hdmi; > > intel_encoder->post_disable = chv_hdmi_post_disable; > > + intel_encoder->post_pll_disable = chv_hdmi_post_pll_disable; > > } else if (IS_VALLEYVIEW(dev)) { > > intel_encoder->pre_pll_enable = vlv_hdmi_pre_pll_enable; > > intel_encoder->pre_enable = vlv_hdmi_pre_enable; > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx