[Intel-gfx] [PATCH] drm/i915: Undo gtt scratch pte unmapping again

[Takashi Iwai]

At Thu, 27 Mar 2014 07:41:41 +0100,
Takashi Iwai wrote:
> 
> At Wed, 26 Mar 2014 20:10:09 +0100,
> Daniel Vetter wrote:
> > 
> > It apparently blows up on some machines. This functionally reverts
> > 
> > commit 828c79087cec61eaf4c76bb32c222fbe35ac3930
> > Author: Ben Widawsky <benjamin.widawsky at intel.com>
> > Date:   Wed Oct 16 09:21:30 2013 -0700
> > 
> >     drm/i915: Disable GGTT PTEs on GEN6+ suspend
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64841
> > Reported-and-Tested-by: Brad  Jackson <bjackson0971 at gmail.com>
> > Cc: stable at vger.kernel.org
> > Cc: Takashi Iwai <tiwai at suse.de>
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Cc: Todd Previte <tprevite at gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> The commit was the fix for the memory corruption and lock up at S4 on
> some (Haswell) machines.  This revert will re-introduce the issue
> again very likely.  I'm going to check with the latest tree, but this
> may take some time.

Luckily I found the machine quickly and tested with your patch.
As expected, this revert breaks S4 again on Haswell machines indeed.
A test machine locks up after 4 times S4 cycle while 3.14-rc8 keeps
working after 100 S4 cycles.  Oh well.


Takashi

> 
> Wouldn't it be safer to revert this conditionally like I suggested in
> comment 10 of the bugzilla entry?
> 
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -818,7 +818,7 @@ void i915_gem_suspend_gtt_mappings(struct
> drm_device *dev)
>         /* Don't bother messing with faults pre GEN6 as we have little
>          * documentation supporting that it's a good idea.
>          */
> -       if (INTEL_INFO(dev)->gen < 6)
> +       if (INTEL_INFO(dev)->gen < 7)
>                 return;
>  
>         i915_check_and_clear_faults(dev);
> 
> thanks,
> 
> Takashi
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 5d61de18ae55..4467974eb53b 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1281,7 +1281,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
> >  	dev_priv->gtt.base.clear_range(&dev_priv->gtt.base,
> >  				       dev_priv->gtt.base.start,
> >  				       dev_priv->gtt.base.total,
> > -				       false);
> > +				       true);
> >  }
> >  
> >  void i915_gem_restore_gtt_mappings(struct drm_device *dev)
> > -- 
> > 1.8.5.2
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>