[Intel-gfx] [PATCH] drm/i915: Hide GEM shutdown in i915_gem_fini

[Chris Wilson]

On Thu, Feb 11, 2016 at 03:39:25PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> As we have i915_gem_init, do the reverse in new i915_gem_fini so
> the fragile order of doing things is hidden from the outside and
> only at one place.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Nick Hoath <nicholas.hoath at intel.com>
> Cc: David Gordon <david.s.gordon at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c |  6 ++----
>  drivers/gpu/drm/i915/i915_drv.h |  2 +-
>  drivers/gpu/drm/i915/i915_gem.c | 40 ++++++++++++++++++++++++----------------
>  3 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2df2fac04708..beda7dea6814 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -444,8 +444,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  
>  cleanup_gem:
>  	mutex_lock(&dev->struct_mutex);
> -	i915_gem_context_fini(dev);
> -	i915_gem_cleanup_engines(dev);
> +	i915_gem_fini(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  cleanup_irq:
>  	intel_guc_ucode_fini(dev);
> @@ -1256,8 +1255,7 @@ int i915_driver_unload(struct drm_device *dev)
>  
>  	intel_guc_ucode_fini(dev);
>  	mutex_lock(&dev->struct_mutex);
> -	i915_gem_context_fini(dev);
> -	i915_gem_cleanup_engines(dev);
> +	i915_gem_fini(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  	intel_fbc_cleanup_cfb(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 090172de81a0..d004dc806670 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3060,11 +3060,11 @@ static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv)
>  void i915_gem_reset(struct drm_device *dev);
>  bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
>  int __must_check i915_gem_init(struct drm_device *dev);
> +void __must_check i915_gem_fini(struct drm_device *dev);
>  int i915_gem_init_rings(struct drm_device *dev);
>  int __must_check i915_gem_init_hw(struct drm_device *dev);
>  int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice);
>  void i915_gem_init_swizzling(struct drm_device *dev);
> -void i915_gem_cleanup_engines(struct drm_device *dev);
>  int __must_check i915_gpu_idle(struct drm_device *dev);
>  int __must_check i915_gem_suspend(struct drm_device *dev);
>  void __i915_add_request(struct drm_i915_gem_request *req,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c63c3072a8a9..f7e7ab432341 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4834,6 +4834,26 @@ cleanup_render_ring:
>  	return ret;
>  }
>  
> +static void
> +i915_gem_cleanup_engines(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_engine_cs *ring;
> +	int i;
> +
> +	for_each_ring(ring, dev_priv, i)
> +		dev_priv->gt.cleanup_ring(ring);
> +
> +	if (i915.enable_execlists) {
> +		/*
> +		 * Neither the BIOS, ourselves or any other kernel
> +		 * expects the system to be in execlists mode on startup,
> +		 * so we need to reset the GPU back to legacy mode.
> +		 */
> +		intel_gpu_reset(dev);
> +	}

This needs to be before contex-fini. If you move this outside of the
mutex, we can then use i915_gpu_reset() and do the full state cleanup.
Similarly that also moves the mutex handling inside i915_gem_fini as
well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre