Re: [PATCH 12/17] drm/i915: Don't kick the power seqeuncer just to check if we have vdd/panel power

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

On Mon, Oct 27, 2014 at 07:10:08PM +0200, Imre Deak wrote:
> On Thu, 2014-10-16 at 21:29 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If there's no power sequencer assigned to the port currently we can't
> > very well have vdd or panel power enabled either. If we would try to
> > check that from the pps registers we'd need to pick a power seqeuncer
> > and kick it. So let's skip the register read and the kick.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 74cf827..c9a1600 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -634,6 +634,10 @@ static bool edp_have_panel_power(struct intel_dp *intel_dp)
> >  
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >  
> > +	if (IS_VALLEYVIEW(dev) &&
> > +	    intel_dp->pps_pipe == INVALID_PIPE)
> > +		return false;
> > +
> >  	return (I915_READ(_pp_stat_reg(intel_dp)) & PP_ON) != 0;
> >  }
> >  
> > @@ -644,6 +648,10 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
> >  
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >  
> > +	if (IS_VALLEYVIEW(dev) &&
> > +	    intel_dp->pps_pipe == INVALID_PIPE)
> > +		return false;
> > +
> 
> During resume this makes intel_edp_panel_vdd_sanitize() think VDD is
> off, though it could be left on by the BIOS. But AFAICS the above makes
> also sure that VDD refcounting won't get broken even in this case. The
> only issue I see is that VDD will be enabled when it was already on,
> which has some overhead, but I don't think that's a priority for now.

Hmm. Yeah, so we should basically do the vlv_initial_power_sequencer_setup()
stuff (or at least some of it) on resume as well. Otherwise the hw and
sw state can get out of sync. I'll see about cooking up a patch for
that...

Oh that makes me think of another issue. What if someone has set
disable_power_wells=0 and then suspends the machine? I think currently
we'd end up leaving the power wells enabled which doesn't sound very
nice, and also it would interfere with the pps_pipe reset handling.
Should we have some kind of "force power wells off" step during suspend?

> 
> >  	return I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD;
> >  }
> >  

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx