Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.

Chris Wilson
2015-07-07T08:58:53Z

On Tue, Jul 07, 2015 at 12:04:10AM +0800, Zhi Wang wrote:
> Hi Chris and Mika:
> Thanks for the comments. I think that reset HW on module unload
> is an good idea. For now I think we should choose a proper position
> in the module unload sequence to reset HW. As GPU reset is render
> engine reset plus ring imrs(They will become to alll F after full
> GPU reset), I think a proper position should be after render and
> interrupt shutdown path.
>
> How about this place?
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index c5349fa..aeaf59e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1133,7 +1133,10 @@ int i915_driver_unload(struct drm_device *dev)
> pm_qos_remove_request(&dev_priv->pm_qos);
>
> i915_global_gtt_cleanup(dev);
> -
> + /* 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);

That feels right. The comment is out-of-place now and needs expansion to
include other side effects for which the gpu reset is meritted.

But this is a riskier patch since we now start doing unconditional
resets for gen3-gen5. Just requires more soak testing, but I would
prefer it as (1) add execlists reset, (2) combine execlists reset +
power context reset into a single unload reset. That way if we do get a
regression in doing the unload reset we can revert back to execlists
easily.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre