Re: S4 resume breakage with i915 driver

David Weinehall
2016-08-26T12:54:41Z

On Fri, Aug 26, 2016 at 01:38:26PM +0100, Chris Wilson wrote:
> On Fri, Aug 26, 2016 at 02:29:41PM +0200, David Weinehall wrote:
> > On Thu, Aug 25, 2016 at 04:32:41PM +0100, Chris Wilson wrote:
> > [snip]
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > > index 3c97f0e7a003..c618bb86aeb9 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > > @@ -574,10 +574,15 @@ mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
> > >
> > > /* These flags are for resource streamer on HSW+ */
> > > if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
> > > - flags |= (HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN);
> > > + flags |= HSW_MI_RS_SAVE_STATE_EN;
> > > else if (INTEL_GEN(dev_priv) < 8)
> > > - flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
> > > -
> > > + flags |= MI_SAVE_EXT_STATE_EN;
> > > + if ((flags & MI_RESTORE_INHIBIT) == 0) {
> > > + if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
> > > + flags |= HSW_MI_RS_RESTORE_STATE_EN;
> > > + else if (INTEL_GEN(dev_priv) < 8)
> > > + flags |= MI_RESTORE_EXT_STATE_EN;
> > > + }
> > >
> > >
> > > But this code is only executed for Haswell, and we only inhibit restore
> > > from uninitialised contexts. The biggest fear is that we are restoring
> > > garbage from userspace and making the GPU do strange things.
> >
> > I notice from the thread that this has already been resolved, but:
> >
> > You say that this is only executed for Haswell?
> >
> > Wouldn't:
> >
> > if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
> >
> > Make it execute on Haswell *or* anything gen8 or newer?
>
> Only if i915.enable_execlists=0.

Ahhhh. Right.

Kind regards, David