Re: [PATCH] drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3)

[Imre Deak <imre.deak@intel.com>]

On to, 2016-01-07 at 10:15 +0200, Jani Nikula wrote:
> On Wed, 06 Jan 2016, Matt Roper <matthew.d.roper@intel.com> wrote:
> > Our attempts save/restore panel power state in i915_suspend.c are
> > causing unclaimed register warnings on BXT since the registers for
> > this
> > platform differ from older platforms.
> > 
> > The big hammer suspend/resume shouldn't be necessary for PP since
> > the
> > connector/encoder hooks should already handle this.  In theory we
> > could
> > remove this for all platforms, but in practice it's likely that
> > would
> > cause some regressions since older platforms with LVDS may have
> > incomplete PP handling.  For now we'll leave the PCH save/restore
> > alone
> > and change the non-PCH branch to only operate on gen <= 4 so that
> > BXT
> > and future platforms aren't included.
> > 
> > v2: Typo fix: s/||/&&/
> > 
> > v3: Change non-PCH condition to a gen <= 4 test rather than listing
> >     VLV/CHV/BXT as specific platforms to exclude; should be more
> >     future-proof as we add new platforms.  (Daniel)
> > 
> > Cc: Vandana Kannan <vandana.kannan@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > Although it's the direction we want to move, I'm not brave enough
> > to blow away
> > the entire PP save/restore for all platforms since I don't have the
> > HW
> > necessary to deal with the regressions that might pop up.  The
> > consensus on IRC
> > is that there probably will be a few regressions when we do that,
> > so I'd rather
> > have people with appropriate platform access make those changes.
> 
> This is the first step we want to take anyway.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Thanks for the patch and review, I pushed the patch to dinq.

--Imre

> 
> 
> 
> > 
> >  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c
> > b/drivers/gpu/drm/i915/i915_suspend.c
> > index a2aa09c..a8af594 100644
> > --- a/drivers/gpu/drm/i915/i915_suspend.c
> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
> > @@ -49,7 +49,7 @@ static void i915_save_display(struct drm_device
> > *dev)
> >  		dev_priv->regfile.savePP_ON_DELAYS =
> > I915_READ(PCH_PP_ON_DELAYS);
> >  		dev_priv->regfile.savePP_OFF_DELAYS =
> > I915_READ(PCH_PP_OFF_DELAYS);
> >  		dev_priv->regfile.savePP_DIVISOR =
> > I915_READ(PCH_PP_DIVISOR);
> > -	} else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> > +	} else if (INTEL_INFO(dev)->gen <= 4) {
> >  		dev_priv->regfile.savePP_CONTROL =
> > I915_READ(PP_CONTROL);
> >  		dev_priv->regfile.savePP_ON_DELAYS =
> > I915_READ(PP_ON_DELAYS);
> >  		dev_priv->regfile.savePP_OFF_DELAYS =
> > I915_READ(PP_OFF_DELAYS);
> > @@ -84,7 +84,7 @@ static void i915_restore_display(struct
> > drm_device *dev)
> >  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv-
> > >regfile.savePP_OFF_DELAYS);
> >  		I915_WRITE(PCH_PP_DIVISOR, dev_priv-
> > >regfile.savePP_DIVISOR);
> >  		I915_WRITE(PCH_PP_CONTROL, dev_priv-
> > >regfile.savePP_CONTROL);
> > -	} else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> > +	} else if (INTEL_INFO(dev)->gen <= 4) {
> >  		I915_WRITE(PP_ON_DELAYS, dev_priv-
> > >regfile.savePP_ON_DELAYS);
> >  		I915_WRITE(PP_OFF_DELAYS, dev_priv-
> > >regfile.savePP_OFF_DELAYS);
> >  		I915_WRITE(PP_DIVISOR, dev_priv-
> > >regfile.savePP_DIVISOR);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx