Re: [PATCH] drm/i915: Re-enable GGTT earlier during resume on pre-gen6 platforms

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, May 10, 2016 at 10:14:19AM +0100, Chris Wilson wrote:
> On Fri, May 06, 2016 at 09:35:55PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Move the intel_enable_gtt() call to happen before we touch the GTT
> > during resume. Right now it's done way too late. Before
> > commit ebb7c78d358b ("agp/intel-gtt: Only register fake agp driver for gen1")
> > it was actually done earlier on account of also getting called from
> > the resume hook of the fake agp driver. With the fake agp driver
> > no longer getting registered we must move the call up.
> > 
> > The symptoms I've seen on my 830 machine include lowmem corruption,
> > other kinds of memory corruption, and straight up hung machine during
> > or just after resume. Not really sure what causes the memory corruption,
> > but so far I've not seen any with this fix.
> > 
> > I think we shouldn't really need to call this during init, but we have
> > been doing that so I've decided to keep the call. However moving that
> > call earlier could be prudent as well. Doing it right after the
> > intel-gtt probe seems appropriate.
> 
> So why not in i915_ggtt_init_hw()? Because you want to keep the current
> semantics of doing it everytime.
>  
> > Also tested this on 946gz,elk,ilk and all seemed quite happy with
> > this change.
> > 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > Fixes: ebb7c78d358b ("agp/intel-gtt: Only register fake agp driver for gen1")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > index 18af3af18754..6e34f2e9f080 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -516,6 +516,7 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
> >  		px_dma(ppgtt->base.scratch_pd);
> >  }
> >  
> > +int i915_ggtt_enable_hw(struct drm_device *dev);
> >  int i915_ggtt_init_hw(struct drm_device *dev);
> 
> Minor nit, probably best to put it after init_hw, since we do init first
> then enable.

Pushed to dinq, with a fresh coat of paint. Thanks for the review.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx