On Mon, Oct 27, 2014 at 04:43:07PM +0200, Imre Deak wrote: > On Thu, 2014-10-16 at 21:27 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Since we read the current power seqeuncer delays from the registers > > (as well as looking at the vbt and spec values) we may end up > > corrupting delays we already initialized when we switch to another > > pipe and the power seqeuncer there has different values currently > > in the registers. > > > > So make sure we only initialize the delays once even if > > intel_dp_init_panel_power_sequencer() gets called multiple times. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 7a10464..9a1295d 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4756,6 +4756,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > lockdep_assert_held(&dev_priv->pps_mutex); > > > > + /* already initialized? */ > > + if (final->t11_t12 != 0) > > + return; > > + > > I wonder if some other place depends on the PP_CONTROL unlocking done > here. At least intel_dp_init_panel_power_sequencer_registers() doesn't > do the unlocking when writing to other PP regs. Maybe the locking > mechanism has an effect only while power sequencing is active, so it > wouldn't matter but the comment in this function suggests that we need > to unlock as a first step. The VLV spec is unclear if unlocking is > needed. Have I mentioned recently how much I _hate_ all of these hardware lockout mechanisms? They just get in the way of doing stuff. Anyway the spec has this to say about the PP_ON bit: "If this bit is not a zero, it activates the register write protect and writes to those registers will be ignored unless the write protect key value is set in the panel sequencing control register." So I think we should be safe. > > > > if (HAS_PCH_SPLIT(dev)) { > > pp_ctrl_reg = PCH_PP_CONTROL; > > pp_on_reg = PCH_PP_ON_DELAYS; > -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx