Re: S4 resume breakage with i915 driver

Imre Deak
2016-08-29T15:03:18Z

On ma, 2016-08-29 at 16:24 +0200, Takashi Iwai wrote:
> On Mon, 29 Aug 2016 16:09:23 +0200,
> Imre Deak wrote:
> >
> > On ma, 2016-08-29 at 15:32 +0200, Daniel Vetter wrote:
> > > On Fri, Aug 26, 2016 at 02:42:47PM +0300, Imre Deak wrote:
> > > > On pe, 2016-08-26 at 14:10 +0300, Imre Deak wrote:
> > > > > On pe, 2016-08-26 at 11:39 +0100, Chris Wilson wrote:
> > > > > > On Fri, Aug 26, 2016 at 12:25:01PM +0200, Takashi Iwai
> > > > > > wrote:
> > > > > > > On Fri, 26 Aug 2016 11:18:00 +0200,
> > > > > > > Takashi Iwai wrote:
> > > > > > > > I had to modify the intel_gpu_reset() call because the
> > > > > > > > test was
> > > > > > > > done
> > > > > > > > on the older kernel, so it's like:
> > > > > > > >
> > > > > > > > +       intel_gpu_reset(dev_to_i915(dev)->dev);
> > > > > > > >
> > > > > > > > And, it seems working on HSW! \o/
> > > > > > > >
> > > > > > > > A simple trick, better than the magical register write
> > > > > > > > revert.
> > > > > > > > I'll check other machines, too, to see whether it has
> > > > > > > > any
> > > > > > > > negative
> > > > > > > > impact.
> > > > > > >
> > > > > > > The test results look good on all machines.
> > > > > >
> > > > > > The theory then is that the GPU's are active across the
> > > > > > load of the
> > > > > > hibernation image and so before the GTT is updated the
> > > > > > memory
> > > > > > currently
> > > > > > in use by the GPU is reused by the system.
> > > > > >
> > > > > > The key question then is the memory of boot kernel still in
> > > > > > place
> > > > > > during
> > > > > > the hibernate restore phase?
> > > > >
> > > > > Before restoring the image all devices are quiesced by
> > > > > calling their
> > > > > freeze callback, so the GPU should be idle already
> > > > > in i915_pm_restore_early() already.
> > > >
> > > > But this happens in the loader kernel, so if that doesn't have
> > > > the
> > > > driver built-in then the freeze callback won't be called
> > > > either. So any
> > > > possible BIOS related GPU activity/setup should be quiesced
> > > > from the
> > > > restore callback then.
> > >
> > > I thought the loader kernel has an entire initrd attached, to
> > > allow stuff
> > > like typing in the disk encryption passwd. Which means we very
> > > much do
> > > load i915 in the loader kernel already.
> >
> > AFAICS, the hibernation image is restored from a late_initcall and
> > so
> > /bin/init etc. won't be run in the loader kernel and so the driver
> > won't be loaded if built as a module.
>
> Well, on many systems, it's explicitly triggered from initrd (at
> least, (open)SUSE does it so since ages ago).  dracut does it after
> the whole driver initializations on initrd, usually.

Right, with manual resume that will work. But it's still possible not
to have the driver configured or use kernel resume (passing resume=..).

> > But in theory at least it's
> > possible that the driver won't even be configured in the loader
> > kernel.
> >
> > > So maybe we need to throw a gpu reset into the right hook
> > > (shutdown or
> > > whatever it was) to make sure the loader kernel really stops all
> > > gpu write
> > > cycles, including anything done due to power saving context
> > > restoring.
> >
> > The callback called right before the hibernation image is restored
> > is
> > freeze. Shutdown is called only after creating the image, before
> > powering off.
>
> Hmm, this always confuses me.  Is the freeze callback called to the
> loader kernel?

It's called both in loader and target kernel, before creating or
restoring the image.

--Imre