Re: [PATCH 02/14] drm/i915: Reorganize vlv eDP reboot notifier

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

On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote:
> On Mon, 18 Aug 2014, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check
> > and flatten the rest of the function.
> 
> Please imagine adding another platform there, and realize this just adds
> unnecessary churn.

I'd just add another reboot notifier then. Frankly I don't understand
the current one either. Why does it need to set the delay to max for
instance? And does this mean that the PANEL_POWER_RESET bit doesn't
actually work as advertised in the docs?

> 
> BR,
> Jani.
> 
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 43dd226..a9ed2a6 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -347,22 +347,22 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 pp_div;
> >  	u32 pp_ctrl_reg, pp_div_reg;
> > -	enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> > +	enum pipe pipe;
> >  
> > -	if (!is_edp(intel_dp) || code != SYS_RESTART)
> > +	if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART)
> >  		return 0;
> >  
> > -	if (IS_VALLEYVIEW(dev)) {
> > -		pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> > -		pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
> > -		pp_div = I915_READ(pp_div_reg);
> > -		pp_div &= PP_REFERENCE_DIVIDER_MASK;
> > +	pipe = vlv_power_sequencer_pipe(intel_dp);
> >  
> > -		/* 0x1F write to PP_DIV_REG sets max cycle delay */
> > -		I915_WRITE(pp_div_reg, pp_div | 0x1F);
> > -		I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> > -		msleep(intel_dp->panel_power_cycle_delay);
> > -	}
> > +	pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> > +	pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
> > +	pp_div = I915_READ(pp_div_reg);
> > +	pp_div &= PP_REFERENCE_DIVIDER_MASK;
> > +
> > +	/* 0x1F write to PP_DIV_REG sets max cycle delay */
> > +	I915_WRITE(pp_div_reg, pp_div | 0x1F);
> > +	I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> > +	msleep(intel_dp->panel_power_cycle_delay);
> >  
> >  	return 0;
> >  }
> > -- 
> > 1.8.5.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC