On Mon, Oct 27, 2014 at 07:10:08PM +0200, Imre Deak wrote: > On Thu, 2014-10-16 at 21:29 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
