[Intel-gfx] [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation

[Chris Wilson]

On Thu, Aug 25, 2016 at 10:15:40AM +0100, Chris Wilson wrote:
> Recently I have been applying an optimisation to avoid stalling and
> clflushing GGTT objects based on their current binding. That is we only
> set-to-gtt-domain upon first bind. However, on hibernation the objects
> remain bound, but they are in the CPU domain. Currently (since commit
> 975f7ff42edf ("drm/i915: Lazily migrate the objects after hibernation"))
> we only flush scanout objects as all other objects are expected to be
> flushed prior to use. That breaks down in the face of the runtime
> optimisation above - and we need to flush all GGTT pinned objects
> (essentially ringbuffers).
> 
> To reduce the burden of extra clflushes, we only flush those objects we
> cannot discard from the GGTT. Everything pinned to the scanout, or
> current contexts or ringbuffers will be flushed and rebound. Other
> objects, such as inactive contexts, will be left unbound and in the CPU
> domain until first use after resuming.
> 
> Fixes: 7abc98fadfdd ("drm/i915: Only change the context object's domain...")
> Fixes: 57e885318119 ("drm/i915: Use VMA for ringbuffer tracking")
> References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: David Weinehall <david.weinehall at intel.com>

Any takers? There's a small risk of failure after hibernate on Baytrail.
(The effect is most likely limited to contexts on !llc, but even then is
going to be rare I think.)

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 570e7311a419..72d03127dec4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3234,7 +3234,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -	struct drm_i915_gem_object *obj;
> +	struct drm_i915_gem_object *obj, *on;
>  	struct i915_vma *vma;
>  
>  	i915_check_and_clear_faults(dev_priv);
> @@ -3243,20 +3243,31 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total,
>  			       true);
>  
> +	ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
> +
>  	/* Cache flush objects bound into GGTT and rebind them. */
> -	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> +	list_for_each_entry_safe(obj, on,
> +				 &dev_priv->mm.bound_list, global_list) {
> +		bool ggtt_bound = false;
> +
>  		list_for_each_entry(vma, &obj->vma_list, obj_link) {
>  			if (vma->vm != &ggtt->base)
>  				continue;
>  
> +			if (!i915_vma_unbind(vma))
> +				continue;
> +
>  			WARN_ON(i915_vma_bind(vma, obj->cache_level,
>  					      PIN_UPDATE));
> +			ggtt_bound = true;
>  		}
>  
> -		if (obj->pin_display)
> +		if (ggtt_bound)
>  			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
>  	}
>  
> +	ggtt->base.closed = false;
> +
>  	if (INTEL_INFO(dev)->gen >= 8) {
>  		if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
>  			chv_setup_private_ppat(dev_priv);
> -- 
> 2.9.3
> 

-- 
Chris Wilson, Intel Open Source Technology Centre