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

[Chris Wilson]

On Fri, May 06, 2016 at 09:35:55PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at 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 at ffwll.ch>
> Cc: drm-intel-fixes at lists.freedesktop.org
> Fixes: ebb7c78d358b ("agp/intel-gtt: Only register fake agp driver for gen1")
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Reviewed-by: Chris Wilson <chris at 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.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre