On Sun, Apr 24, 2016 at 07:10:19PM +0100, Chris Wilson wrote:
> 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).
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> CC: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Cc: "Niu, Bing" <bing.niu@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 41 ++++++++++++++++++++++++++-------
> drivers/gpu/drm/i915/i915_gem.c | 8 -------
> drivers/gpu/drm/i915/i915_gem_context.c | 8 +------
> 3 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5c7615041b31..68c60f4fd44d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -425,6 +425,37 @@ 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)
> +{
> + int reset;
> +
> + /*
> + * 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.
> + */
> + reset = intel_gpu_reset(dev, ALL_ENGINES);
That's going to kill pre-g4x macines I think. At least until Maarten's
reset locking fixes go in. Even then I'm not sure we want to do that on
those old machines.
> + 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;
> @@ -506,10 +537,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);
> @@ -1456,10 +1484,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 a88aa7f11a9c..82e41eeb36bb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4970,14 +4970,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 fef33a3ce3b7..26122de365b2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -452,14 +452,8 @@ void i915_gem_context_fini(struct drm_device *dev)
>
> 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.8.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
