* [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
@ 2015-07-03 16:27 bing.niu
2015-07-03 9:01 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: bing.niu @ 2015-07-03 16:27 UTC (permalink / raw)
To: intel-gfx; +Cc: bing.niu
From: "Niu,Bing" <bing.niu@intel.com>
It is found that i915 will not reset gpu under execlist mode when
unload module. that will lead to some issues when unload/load module
with different submission mode. e.g. from execlist mode to ring
buffer mode via loading/unloading i915. Because HW is not in a reset
state and registers are not clean under such condition.
Signed-off-by: Niu,Bing <bing.niu@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index be35f04..7855dd3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5115,6 +5115,14 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
for_each_ring(ring, dev_priv, i)
dev_priv->gt.cleanup_ring(ring);
+
+ 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);
}
static void
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-03 16:27 [PATCH v2] drm/i915: Also perform gpu reset under execlist mode bing.niu
@ 2015-07-03 9:01 ` Chris Wilson
2015-07-03 10:52 ` Mika Kuoppala
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2015-07-03 9:01 UTC (permalink / raw)
To: bing.niu; +Cc: intel-gfx
On Sat, Jul 04, 2015 at 12:27:34AM +0800, bing.niu@intel.com wrote:
> From: "Niu,Bing" <bing.niu@intel.com>
>
> It is found that i915 will not reset gpu under execlist mode when
> unload module. that will lead to some issues when unload/load module
> with different submission mode. e.g. from execlist mode to ring
> buffer mode via loading/unloading i915. Because HW is not in a reset
> state and registers are not clean under such condition.
>
> Signed-off-by: Niu,Bing <bing.niu@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
I think we may end up doing the reset unconditionally in
i915_driver_unload() because this argument holds for almost everything
we setup. It's a bigger risk because of doing the gpu-reset on more
machines, but module-unloading is a "developer feature"!
The only issue is making sure that the reset is ordered appropriately.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-03 9:01 ` Chris Wilson
@ 2015-07-03 10:52 ` Mika Kuoppala
2015-07-06 16:04 ` Zhi Wang
0 siblings, 1 reply; 12+ messages in thread
From: Mika Kuoppala @ 2015-07-03 10:52 UTC (permalink / raw)
To: Chris Wilson, bing.niu; +Cc: intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> On Sat, Jul 04, 2015 at 12:27:34AM +0800, bing.niu@intel.com wrote:
>> From: "Niu,Bing" <bing.niu@intel.com>
>>
>> It is found that i915 will not reset gpu under execlist mode when
>> unload module. that will lead to some issues when unload/load module
>> with different submission mode. e.g. from execlist mode to ring
>> buffer mode via loading/unloading i915. Because HW is not in a reset
>> state and registers are not clean under such condition.
>>
>> Signed-off-by: Niu,Bing <bing.niu@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> I think we may end up doing the reset unconditionally in
> i915_driver_unload() because this argument holds for almost everything
> we setup. It's a bigger risk because of doing the gpu-reset on more
> machines, but module-unloading is a "developer feature"!
And after that has been sorted, we should try reset on module load.
This way initial state would be identical to after reset/unload state.
Now we have this situation that we don't know how much we are leaning on
bios on state setup.
-Mika
> The only issue is making sure that the reset is ordered appropriately.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-03 10:52 ` Mika Kuoppala
@ 2015-07-06 16:04 ` Zhi Wang
2015-07-07 8:58 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2015-07-06 16:04 UTC (permalink / raw)
To: Mika Kuoppala, Chris Wilson, bing.niu; +Cc: intel-gfx
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);
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/03/15 18:52, Mika Kuoppala 写道:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
>> On Sat, Jul 04, 2015 at 12:27:34AM +0800, bing.niu@intel.com wrote:
>>> From: "Niu,Bing" <bing.niu@intel.com>
>>>
>>> It is found that i915 will not reset gpu under execlist mode when
>>> unload module. that will lead to some issues when unload/load module
>>> with different submission mode. e.g. from execlist mode to ring
>>> buffer mode via loading/unloading i915. Because HW is not in a reset
>>> state and registers are not clean under such condition.
>>>
>>> Signed-off-by: Niu,Bing <bing.niu@intel.com>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> I think we may end up doing the reset unconditionally in
>> i915_driver_unload() because this argument holds for almost everything
>> we setup. It's a bigger risk because of doing the gpu-reset on more
>> machines, but module-unloading is a "developer feature"!
>
> And after that has been sorted, we should try reset on module load.
>
> This way initial state would be identical to after reset/unload state.
> Now we have this situation that we don't know how much we are leaning on
> bios on state setup.
>
> -Mika
>
>> The only issue is making sure that the reset is ordered appropriately.
>> -Chris
>>
>> --
>> Chris Wilson, Intel Open Source Technology Centre
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-06 16:04 ` Zhi Wang
@ 2015-07-07 8:58 ` Chris Wilson
2015-07-06 19:38 ` Zhi Wang
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2015-07-07 8:58 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx, bing.niu
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-07 8:58 ` Chris Wilson
@ 2015-07-06 19:38 ` Zhi Wang
2015-07-07 11:17 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2015-07-06 19:38 UTC (permalink / raw)
To: Chris Wilson, Mika Kuoppala, bing.niu, intel-gfx
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-06 19:38 ` Zhi Wang
@ 2015-07-07 11:17 ` Chris Wilson
2015-07-06 20:23 ` Zhi Wang
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2015-07-07 11:17 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx, bing.niu
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
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-07 11:17 ` Chris Wilson
@ 2015-07-06 20:23 ` Zhi Wang
2015-07-07 12:00 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2015-07-06 20:23 UTC (permalink / raw)
To: Chris Wilson, Mika Kuoppala, bing.niu, intel-gfx
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-06 20:23 ` Zhi Wang
@ 2015-07-07 12:00 ` Chris Wilson
2015-07-06 20:32 ` Zhi Wang
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2015-07-07 12:00 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx, bing.niu
On Tue, Jul 07, 2015 at 04:23:13AM +0800, Zhi Wang wrote:
> 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
I'd be happy to r-b that. Having a verbose comment explaining why we put
the shotgun to our head is definitely required, thanks.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-07 12:00 ` Chris Wilson
@ 2015-07-06 20:32 ` Zhi Wang
2015-07-07 12:10 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2015-07-06 20:32 UTC (permalink / raw)
To: Chris Wilson, Mika Kuoppala, bing.niu, intel-gfx
Thanks for the comments!:) I'm wondering that why the GEN>=5 is needed
here. Is there any HW problems in GEN>=5 when driver wants to do GPU
reset? I'm sorry that I don't know much about GEN<6...Just want to know
more about the story:)
于 07/07/15 20:00, Chris Wilson 写道:
> On Tue, Jul 07, 2015 at 04:23:13AM +0800, Zhi Wang wrote:
>> 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
>
> I'd be happy to r-b that. Having a verbose comment explaining why we put
> the shotgun to our head is definitely required, thanks.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-06 20:32 ` Zhi Wang
@ 2015-07-07 12:10 ` Chris Wilson
2015-07-06 20:53 ` Zhi Wang
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2015-07-07 12:10 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx, bing.niu
On Tue, Jul 07, 2015 at 04:32:53AM +0800, Zhi Wang wrote:
> Thanks for the comments!:) I'm wondering that why the GEN>=5 is
> needed here. Is there any HW problems in GEN>=5 when driver wants to
> do GPU reset? I'm sorry that I don't know much about GEN<6...Just
> want to know more about the story:)
gen5 is about the earliest that I know the hw stated to get smart and
doing automatic power saving by writing to pinned buffers. Before gen5
our reset hasn't been very reliable, and only recently have we had code
that is meant to be able to reset the gpu on those gen - hence my
reservation about doing so unconditionally.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Also perform gpu reset under execlist mode.
2015-07-07 12:10 ` Chris Wilson
@ 2015-07-06 20:53 ` Zhi Wang
0 siblings, 0 replies; 12+ messages in thread
From: Zhi Wang @ 2015-07-06 20:53 UTC (permalink / raw)
To: Chris Wilson, Mika Kuoppala, bing.niu, intel-gfx
Thanks! I had question about power context.
I was curious about HW context and once dumped the GEM context object on
GEN6 before.
According to B-spec, Gen Graphics » BSpec » 3D-Media-GPGPU Engine »
Registers in Render Engine » Render Logical Context Data » Register
State Context [SNB+] » Register State Context [HSW] »
I thought the context should contain all the register regions in the
list include the dark yellow marked registers as power context
registers, but I saw the content of the context I got began with main
context, LRI signature 110010bf. .. It's weird.
I don't know if the context saved by MI_SET_CONTEXT is just the power
context in B-spec?If yes, why doest it looks like that?
Thanks,
Zhi.
于 07/07/15 20:10, Chris Wilson 写道:
> On Tue, Jul 07, 2015 at 04:32:53AM +0800, Zhi Wang wrote:
>> Thanks for the comments!:) I'm wondering that why the GEN>=5 is
>> needed here. Is there any HW problems in GEN>=5 when driver wants to
>> do GPU reset? I'm sorry that I don't know much about GEN<6...Just
>> want to know more about the story:)
>
> gen5 is about the earliest that I know the hw stated to get smart and
> doing automatic power saving by writing to pinned buffers. Before gen5
> our reset hasn't been very reliable, and only recently have we had code
> that is meant to be able to reset the gpu on those gen - hence my
> reservation about doing so unconditionally.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-07 12:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-03 16:27 [PATCH v2] drm/i915: Also perform gpu reset under execlist mode bing.niu
2015-07-03 9:01 ` Chris Wilson
2015-07-03 10:52 ` Mika Kuoppala
2015-07-06 16:04 ` Zhi Wang
2015-07-07 8:58 ` Chris Wilson
2015-07-06 19:38 ` Zhi Wang
2015-07-07 11:17 ` Chris Wilson
2015-07-06 20:23 ` Zhi Wang
2015-07-07 12:00 ` Chris Wilson
2015-07-06 20:32 ` Zhi Wang
2015-07-07 12:10 ` Chris Wilson
2015-07-06 20:53 ` Zhi Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox