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

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

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?

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

         i915_global_gtt_cleanup(dev);

+       /*
+        * Restore HW workload submission mode back to default mode when 
shutdown.
+        * 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 
mode is GPU reset.
+        */
+       if (i915.enable_execlists)
+               intel_gpu_reset(dev);
+
         intel_uncore_fini(dev);
         if (dev_priv->regs != NULL)
                 pci_iounmap(dev->pdev, dev_priv->regs);

于 07/07/15 16:58, Chris Wilson 写道:
> 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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx