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(®->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
