From e7ae86bab90887e1892e6c33c4fb38d8ca6935f9 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 28 Apr 2016 09:56:59 +0100 Subject: [PATCH] drm/i915: Unify GPU resets upon shutdown Both execlists and legacy need to reset the context (and mode) of the GPU before we lose control of the system. By resetting the GPU, we revert back to default settings. This simplifies the life of any subsequent driver (in particular for virtualized setups) as it does not then have to try and recover from an unknown condition. As both paths need to reset for the same reason, move the reset to a common point. This unifies the resets added in a647828afc (drm/i915: Also perform gpu reset under execlist mode) and 8e96d9c4d9 (drm/i915: reset the GPU on context fini). v2: Restrict the reset to "modern" gen (where we enable HW contexts) to try and avoid leaving the machine in an unusable state with a risky reset on older GPU. This should keep the status quo as to who performs resets (i.e. currently only GPUs with HW contexts perform a reset on shutdown). Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin CC: Joonas Lahtinen Cc: Mika Kuoppala Cc: "Niu, Bing" Reviewed-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1461833819-3991-25-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_dma.c | 45 ++++++++++++++++++++----- drivers/gpu/drm/i915/i915_gem.c | 8 ----- drivers/gpu/drm/i915/i915_gem_context.c | 10 +----- 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index fd1260d2b6ec..f69330cf0118 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -425,6 +425,41 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = { .can_switch = i915_switcheroo_can_switch, }; +static void i915_gem_fini(struct drm_device *dev) +{ + /* + * 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. And the only + * known way to disable logical contexts is through a GPU reset. + * + * So in order to leave the system in a known default configuration, + * always reset the GPU upon unload. Afterwards we then clean up the + * GEM state tracking, flushing off the requests and leaving the + * system in a known idle state. + * + * Note that is of the upmost importance that the GPU is idle and + * all stray writes are flushed *before* we dismantle the backing + * storage for the pinned objects. + * + * However, since we are uncertain that reseting the GPU on older + * machines is a good idea, we don't - just in case it leaves the + * machine in an unusable condition. + */ + if (HAS_HW_CONTEXTS(dev)) { + int reset = intel_gpu_reset(dev, ALL_ENGINES); + WARN_ON(reset && reset != -ENODEV); + } + + mutex_lock(&dev->struct_mutex); + i915_gem_reset(dev); + i915_gem_cleanup_engines(dev); + i915_gem_context_fini(dev); + mutex_unlock(&dev->struct_mutex); + + WARN_ON(!list_empty(&to_i915(dev)->context_list)); +} + static int i915_load_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -509,10 +544,7 @@ static int i915_load_modeset_init(struct drm_device *dev) return 0; cleanup_gem: - mutex_lock(&dev->struct_mutex); - i915_gem_cleanup_engines(dev); - i915_gem_context_fini(dev); - mutex_unlock(&dev->struct_mutex); + i915_gem_fini(dev); cleanup_irq: intel_guc_ucode_fini(dev); drm_irq_uninstall(dev); @@ -1459,10 +1491,7 @@ int i915_driver_unload(struct drm_device *dev) flush_workqueue(dev_priv->wq); intel_guc_ucode_fini(dev); - mutex_lock(&dev->struct_mutex); - i915_gem_cleanup_engines(dev); - i915_gem_context_fini(dev); - mutex_unlock(&dev->struct_mutex); + i915_gem_fini(dev); intel_fbc_cleanup_cfb(dev_priv); intel_power_domains_fini(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 411201f94b43..e9841eab5e35 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4969,14 +4969,6 @@ i915_gem_cleanup_engines(struct drm_device *dev) for_each_engine(engine, dev_priv) dev_priv->gt.cleanup_engine(engine); - - 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, ALL_ENGINES); } static void diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 25a9a1445ba9..b1b704c2c001 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -450,16 +450,8 @@ void i915_gem_context_fini(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_context *dctx = dev_priv->kernel_context; - i915_gem_context_lost(dev_priv); - - if (dctx->legacy_hw_ctx.rcs_state) { - /* 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, ALL_ENGINES); - + if (dctx->legacy_hw_ctx.rcs_state) i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state); - } i915_gem_context_unreference(dctx); dev_priv->kernel_context = NULL; -- 2.20.1