[Intel-gfx] [PATCH 05/17] drm/i915: Don't initialize power seqeuencer delays more than once

[Daniel Vetter]

On Mon, Oct 27, 2014 at 04:55:16PM +0200, Ville Syrjälä wrote:
> On Mon, Oct 27, 2014 at 04:43:07PM +0200, Imre Deak wrote:
> > On Thu, 2014-10-16 at 21:27 +0300, ville.syrjala at linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > 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 at linux.intel.com>
> > > ---
> > >  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."

I've added this quote to the patch when applying, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch