On Fri, 30 May 2014 23:12:45 +0200
"Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx> wrote:
> On Friday, May 30, 2014 11:29:15 AM Jesse Barnes wrote:
> > On Fri, 30 May 2014 16:37:53 +0300
> > Imre Deak <imre.deak@xxxxxxxxx> wrote:
> >
> > > On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > > > From: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>
> > > >
> > > > 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 <kristen@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> > > > ---
> > > > 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.deak@xxxxxxxxx>
> >
> > 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.
Ok thanks. Kristen corrected me on IRC too. The latest patch I sent
should do what we want then, now that I've removed the freeze_late
function and put our PC8 enable in suspend_late like Imre suggested
initially.
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
