On Friday, May 30, 2014 11:29:15 AM Jesse Barnes wrote:
> On Fri, 30 May 2014 16:37:53 +0300
> Imre Deak <imre.d...@intel.com> wrote:
>
> > On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > > From: Kristen Carlson Accardi <kris...@linux.intel.com>
> > >
> > > This matches the runtime suspend paths and allows the system to enter
> > > the lowest power mode at freeze time.
> > >
> > > Signed-off-by: Kristen Carlson Accardi <kris...@linux.intel.com>
> > > Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index b6211d7..24dc856 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
> > >
> > > intel_display_set_init_power(dev_priv, false);
> > >
> > > + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > > + hsw_enable_pc8(dev_priv);
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev,
> > > bool restore_gtt_mappings)
> > > {
> > > struct drm_i915_private *dev_priv = dev->dev_private;
> > >
> > > + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > > + hsw_disable_pc8(dev_priv);
> >
> > I would put this before we access any of the HW regs in thaw_early() and
> > correspondingly the above call to hsw_enable_pc8() to suspend_late()
> > before we call pci_disable_device().
> >
> > With that change this is:
> > Reviewed-by: Imre Deak <imre.d...@intel.com>
>
> For the thaw side I think that makes sense.
>
> But for the freeze side, putting it in suspend_late won't get us the
> freeze behavior we want. I think Rafael and Kristen are thinking of
> re-using the freeze infrastructure for some kind of connected standby
> feature, where most stuff is frozen, but the system isn't in S3 or S4,
> so we need the enable_pc8 call in the _freeze path as well.
>
> Rafael, is that correct?
No, it isn't. The .freeze()/.thaw() callbacks are hibernation-specific.
There are no plans for using this in PM beyond hibernation.
What we're going to use are .suspend/_late/_noirq() and the corresponding
resume callbacks and runtime PM.
> I'll add a late_freeze and put it there instead, so it doesn't pollute
> the S3 suspend path.
The freeze/thaw stuff need not do any PM BTW, it just needs to quiesce the
device to prevent it from doing DMA etc and then bring it back to life.
Rafael
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
