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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx