[Intel-gfx] [PATCH] drm/i915: Restore fences after resume and GPU resets

[Daniel Vetter]

On Wed, Jun 12, 2013 at 10:15:12AM +0100, Chris Wilson wrote:
> Stéphane Marchesin found that fences for pinned objects (i.e. the
> scanout) were not being restored upon resume, leading to corruption on
> the display and reference counting issues. This is due to a bug in
> 
> commit 312817a39f17dbb4de000165b5b724e3728cd91c [2.6.38]
> Author: Chris Wilson <chris at chris-wilson.co.uk>
> Date:   Mon Nov 22 11:50:11 2010 +0000
> 
>     drm/i915: Only save and restore fences for UMS
> 
> that zapped the pinned fences even though they were in use.
> Fortuitously, whilst we forced a VT switch during suspend and resume,
> no fences were ever pinned at the time. However, we now can do
> switchless S3 transitions and so the old bug finally surfaces.
> 
> Reported-by: Stéphane Marchesin <marcheu at chromium.org>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Stéphane Marchesin <marcheu at chromium.org>

Now that people have hit the WARN, picked up for -fixes, thanks for the
patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |    2 ++
>  drivers/gpu/drm/i915/i915_gem.c     |   22 +++++-----------------
>  drivers/gpu/drm/i915/i915_suspend.c |    1 +
>  3 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bacc2fe..1a67216 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1793,6 +1793,8 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
>  struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
>  				struct drm_gem_object *gem_obj, int flags);
>  
> +void i915_gem_restore_fences(struct drm_device *dev);
> +
>  /* i915_gem_context.c */
>  void i915_gem_context_init(struct drm_device *dev);
>  void i915_gem_context_fini(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 61956a1..773c8f4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2142,25 +2142,15 @@ static void i915_gem_reset_ring_lists(struct drm_i915_private *dev_priv,
>  		ring->sync_seqno[i] = 0;
>  }
>  
> -static void i915_gem_reset_fences(struct drm_device *dev)
> +void i915_gem_restore_fences(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int i;
>  
>  	for (i = 0; i < dev_priv->num_fence_regs; i++) {
>  		struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
> -
> -		if (reg->obj)
> -			i915_gem_object_fence_lost(reg->obj);
> -
> -		i915_gem_write_fence(dev, i, NULL);
> -
> -		reg->pin_count = 0;
> -		reg->obj = NULL;
> -		INIT_LIST_HEAD(&reg->lru_list);
> +		i915_gem_write_fence(dev, i, reg->obj);
>  	}
> -
> -	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
>  }
>  
>  void i915_gem_reset(struct drm_device *dev)
> @@ -2187,8 +2177,7 @@ void i915_gem_reset(struct drm_device *dev)
>  		obj->base.read_domains &= ~I915_GEM_GPU_DOMAINS;
>  	}
>  
> -	/* The fence registers are invalidated so clear them out */
> -	i915_gem_reset_fences(dev);
> +	i915_gem_restore_fences(dev);
>  }
>  
>  /**
> @@ -3922,8 +3911,6 @@ i915_gem_idle(struct drm_device *dev)
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		i915_gem_evict_everything(dev);
>  
> -	i915_gem_reset_fences(dev);
> -
>  	/* Hack!  Don't let anybody do execbuf while we don't control the chip.
>  	 * We need to replace this with a semaphore, or something.
>  	 * And not confound mm.suspended!
> @@ -4259,7 +4246,8 @@ i915_gem_load(struct drm_device *dev)
>  		dev_priv->num_fence_regs = 8;
>  
>  	/* Initialize fence registers to zero */
> -	i915_gem_reset_fences(dev);
> +	INIT_LIST_HEAD(&dev_priv->mm.fence_list);
> +	i915_gem_restore_fences(dev);
>  
>  	i915_gem_detect_bit_6_swizzle(dev);
>  	init_waitqueue_head(&dev_priv->pending_flip_queue);
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 88b9a66..70db618 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -394,6 +394,7 @@ int i915_restore_state(struct drm_device *dev)
>  
>  	mutex_lock(&dev->struct_mutex);
>  
> +	i915_gem_restore_fences(dev);
>  	i915_restore_display(dev);
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
> -- 
> 1.7.10.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch