Re: [PATCH] drm/i915: fix module unload after context merge

[Ben Widawsky <ben@bwidawsk.net>]

On Tue, 19 Jun 2012 21:55:32 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> commit 8e96d9c4d9843f00ebeb4a9b33596d96602ea101
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Mon Jun 4 14:42:56 2012 -0700
> 
>     drm/i915: reset the GPU on context fini
> 
> broke module unload because it reset the gpu before we've stopped
> touching it. Later on in the unload sequence the ringbuffer code
> complained that the gpu would idle properly (because intel_gpu_reset
> only resets the hw and not our sw state).
> 
> v2: Reorder things so that we reset the gpu _before_ we release the
> backing storage of the default context.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51183
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_dma.c         |    2 +-
>  drivers/gpu/drm/i915/i915_gem_context.c |    7 +++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f1544c5..efba4e8 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1669,7 +1669,6 @@ int i915_driver_unload(struct drm_device *dev)
>  	if (ret)
>  		DRM_ERROR("failed to idle hardware: %d\n", ret);
>  	i915_gem_retire_requests(dev);
> -	i915_gem_context_fini(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	/* Cancel the retire work handler, which should be idle now. */
> @@ -1720,6 +1719,7 @@ int i915_driver_unload(struct drm_device *dev)
>  		mutex_lock(&dev->struct_mutex);
>  		i915_gem_free_all_phys_object(dev);
>  		i915_gem_cleanup_ringbuffer(dev);
> +		i915_gem_context_fini(dev);
>  		mutex_unlock(&dev->struct_mutex);
>  		i915_gem_cleanup_aliasing_ppgtt(dev);
>  		i915_gem_cleanup_stolen(dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8fb8cd8..48e41df 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -277,11 +277,14 @@ void i915_gem_context_fini(struct drm_device *dev)
>  	if (dev_priv->hw_contexts_disabled)
>  		return;
>  
> +	/* The only known way to stop the gpu from accessing the hw context is
> +	 * to reset it. Do this as the very last operation to avoid confusing
> +	 * other code, leading to spurious errors. */
> +	intel_gpu_reset(dev);
> +
>  	i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj);
>  
>  	do_destroy(dev_priv->ring[RCS].default_context);
> -
> -	intel_gpu_reset(dev);
>  }
>  
>  void i915_gem_context_open(struct drm_device *dev, struct drm_file *file)



-- 
Ben Widawsky, Intel Open Source Technology Center