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

Daniel Vetter
2014-10-28T08:26:15Z

On Tue, Oct 28, 2014 at 10:03:10AM +0200, Ville Syrjälä wrote:
> 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...

Yeah, pretty much same thought here. We even have a special callback for
this stuff with encoder->reset!
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch