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

[Zhi Wang <zhi.a.wang@intel.com>]

Hi Chris:

How about this one? :)

diff --git a/drivers/gpu/drm/i915/i915_dma.c 
b/drivers/gpu/drm/i915/i915_dma.c
index c5349fa..013039e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1134,6 +1134,21 @@ int i915_driver_unload(struct drm_device *dev)

         i915_global_gtt_cleanup(dev);

+       /*
+        * The only known way to stop the gpu from accessing the hw 
context in
+        * graphics memory space is to reset it. Do this as the very last
+        * operation to avoid confusing other code, leading to spurious 
errors.
+        *
+        * Besides, we also need to restore HW workload submission mode back
+        * to default mode when shutdown i915.
+        *
+        * It makes i915 module loading/unloading be able to switch between
+        * different workload submission mode on gen8+. And according to 
B-spec,
+        * the only way to reset HW workload submission mode to default 
mod is GPU reset.
+        */
+       if (INTEL_INFO(dev)->gen >= 5)
+               intel_gpu_reset(dev);
+
         intel_uncore_fini(dev);
         if (dev_priv->regs != NULL)
                 pci_iounmap(dev->pdev, dev_priv->regs);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index a7e58a8..376ee6b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -373,11 +373,6 @@ void i915_gem_context_fini(struct drm_device *dev)
         int i;

         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);
-
                 /* When default context is created and switched to, 
base object refcount
                  * will be 2 (+1 from object creation and +1 from 
do_switch()).
                  * i915_gem_context_fini() will be called after 
gpu_idle() has switched


于 07/07/15 19:17, Chris Wilson 写道:
> On Tue, Jul 07, 2015 at 03:38:37AM +0800, Zhi Wang wrote:
>> Hi Chris:
>>      Thanks for the comments! I can understand that we're concerned
>> about regressions, so this is why I think put this reset in module
>> unload path looks much safer. For safety, maybe we should only reset
>> GPU perhaps only when GEN >= 6? That looks much easier and safer,
>> also combine execlist reset and power context reset.
>>
>> Or we just add this before i915_uncore_fini() inside
>> i915_driver_unload()? This way looks much safer?
>>
>> How about this one?
>
> No, if we are just targetting execlists, then disabling it in
> cleanup_ringbuffers as before is the cleanest (as that is the opposite
> stage to where we enable them).
>
> The reset in i915_driver_unload() is preferred to replace all the resets
> required during unload. It is safe to move the context reset here as we
> do not disturb the GTT state between unpining the context and here.
> Making it conditional on gen>=5 is probably a good first step.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx