[Intel-gfx] [PATCH] drm/i915: After hibernation, discard the unbound list

[Ben Widawsky]

On Thu, Feb 07, 2013 at 10:27:47AM +0000, Chris Wilson wrote:
> The unbound list is an optimisation to track objects that have been
> evicted from the GTT but remain untouched by the CPU. After hibernation,
> all memory is in the CPU domain (having been restored from a disk image)
> and so we need to restore the domain tracking upon the objects. However,
> for the unbound list we can simply discard those objects and lazily wait
> for them to be reused.
> 
> v2: Perform the unbound discard explicitly during thawing after
> hibernation.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9069e71..f5ccb3d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -592,6 +592,30 @@ static int __i915_drm_thaw(struct drm_device *dev)
>  	return error;
>  }
>  
> +static void i915_gem_discard_unbound(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *obj, *next;
> +
> +	/* After hibernation all memory is in the CPU domain and so we need
> +	 * to restore our domain tracking. However, the unbound list is
> +	 * merely an optimisation to track memory that has been evicted from
> +	 * the GTT but remains in the GTT domain. After hibernation, that
> +	 * is no longer the case and we can trim the unbound list.
> +	 */
> +	list_for_each_entry_safe(obj, next,
> +				 &dev_priv->mm.unbound_list, gtt_list) {
> +		obj->base.read_domains = I915_GEM_DOMAIN_CPU;
> +		obj->base.write_domain = I915_GEM_DOMAIN_CPU;
> +		if (i915_gem_object_put_pages(obj)) {
> +			/* Abandon hope all ye who enter here */
> +			obj->base.read_domains = I915_GEM_DOMAIN_GTT;
> +			obj->base.write_domain = I915_GEM_DOMAIN_GTT;
> +			/* Will be clflushed during restore_gtt_mappings */
> +		}
> +	}
> +}
> +

I'm confused by how you're going about this. To reorder what you said,
if an object is on the unbound list the GPU is done with it, and it may
require clflushing if on a non-LLC system.

Then a hibernate comes along. Almost certainly the PM subsystem would
have to invalidate all the caches before moving the image to disk.

Assuming the above is true:
First question, why do you want to i915_gem_object_put_pages(). This
would seem to do a bunch of unnecessary stuff (primarily setting pages
as dirty when they need not be).
and second, why do you set I915_GEM_DOMAIN_GTT when put_pages fails?
Is that just a hack to get the clflush?

I think if you can convince me why we need to clflush after resume, I'd
be happy.

>  static int i915_drm_thaw(struct drm_device *dev)
>  {
>  	int error = 0;
> @@ -600,6 +624,7 @@ static int i915_drm_thaw(struct drm_device *dev)
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>  		mutex_lock(&dev->struct_mutex);
> +		i915_gem_discard_unbound(dev);
>  		i915_gem_restore_gtt_mappings(dev);
>  		mutex_unlock(&dev->struct_mutex);
>  	}
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center