* [PATCH 1/3] drm/i915/guc: Rename the function that resets the GuC
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
@ 2017-10-30 18:56 ` Michel Thierry
2017-10-30 21:02 ` Chris Wilson
2017-10-30 18:56 ` [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands Michel Thierry
` (5 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Michel Thierry @ 2017-10-30 18:56 UTC (permalink / raw)
To: intel-gfx
intel_guc_reset sounds more like the microcontroller is the one performing
a reset, while in this case is the opposite. intel_reset_guc not only
makes it clearer, it follows the other intel_reset functions available.
v2: Print error message in English (Tvrtko).
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_uc.c | 4 ++--
drivers/gpu/drm/i915/intel_uncore.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d760f995a22a..0ed06ca07568 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3325,7 +3325,7 @@ extern int i915_reset_engine(struct intel_engine_cs *engine,
unsigned int flags);
extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv);
-extern int intel_guc_reset(struct drm_i915_private *dev_priv);
+extern int intel_reset_guc(struct drm_i915_private *dev_priv);
extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
extern void intel_hangcheck_init(struct drm_i915_private *dev_priv);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 25bd162f38d2..aec295470e0d 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -33,9 +33,9 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
int ret;
u32 guc_status;
- ret = intel_guc_reset(dev_priv);
+ ret = intel_reset_guc(dev_priv);
if (ret) {
- DRM_ERROR("GuC reset failed, ret = %d\n", ret);
+ DRM_ERROR("Failed to reset GuC, ret = %d\n", ret);
return ret;
}
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 96ee6b2754be..d629b2e40013 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1803,7 +1803,7 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
i915_modparams.reset >= 2);
}
-int intel_guc_reset(struct drm_i915_private *dev_priv)
+int intel_reset_guc(struct drm_i915_private *dev_priv)
{
int ret;
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
2017-10-30 18:56 ` [PATCH 1/3] drm/i915/guc: Rename the function that resets the GuC Michel Thierry
@ 2017-10-30 18:56 ` Michel Thierry
2017-10-30 20:58 ` Chris Wilson
` (2 more replies)
2017-10-30 18:56 ` [PATCH 3/3] HAX enable GuC submission for CI Michel Thierry
` (4 subsequent siblings)
6 siblings, 3 replies; 22+ messages in thread
From: Michel Thierry @ 2017-10-30 18:56 UTC (permalink / raw)
To: intel-gfx
This patch adds per engine reset and recovery (TDR) support when GuC is
used to submit workloads to GPU.
In the case of i915 directly submission to ELSP, driver manages hang
detection, recovery and resubmission. With GuC submission these tasks
are shared between driver and GuC. i915 is still responsible for detecting
a hang, and when it does it only requests GuC to reset that Engine. GuC
internally manages acquiring forcewake and idling the engine before actually
resetting it.
Once the reset is successful, i915 takes over again and handles resubmission.
The scheduler in i915 knows which requests are pending so after resetting
a engine, pending workloads/requests are resubmitted again.
v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
non-guc funtion names.
v3: Removed debug message about engine restarting from which request,
since the new baseline do it regardless of submission mode. (Chris)
v4: Rebase.
v5: Do not pass unnecessary reporting flags to the fw (Jeff);
tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 9 +++++++--
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_guc.c | 24 ++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_guc_fwif.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 5 -----
5 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index af745749509c..02fb35744f66 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1984,10 +1984,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
goto out;
}
- ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
+ if (!engine->i915->guc.execbuf_client)
+ ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
+ else
+ ret = intel_guc_reset_engine(engine);
+
if (ret) {
/* If we fail here, we expect to fallback to a global reset */
- DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n",
+ DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
+ (engine->i915->guc.execbuf_client ? "GUC " : ""),
engine->name, ret);
goto out;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0ed06ca07568..b77fd9720ec2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3326,6 +3326,7 @@ extern int i915_reset_engine(struct intel_engine_cs *engine,
extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv);
extern int intel_reset_guc(struct drm_i915_private *dev_priv);
+extern int intel_guc_reset_engine(struct intel_engine_cs *engine);
extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
extern void intel_hangcheck_init(struct drm_i915_private *dev_priv);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index f74d50fdaeb0..f7b9b6b91499 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -24,6 +24,7 @@
#include "intel_guc.h"
#include "i915_drv.h"
+#include "i915_guc_submission.h"
static void gen8_guc_raise_irq(struct intel_guc *guc)
{
@@ -283,6 +284,29 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
+/**
+ * intel_guc_reset_engine() - ask GuC to reset an engine
+ * @engine: engine to be reset
+ */
+int intel_guc_reset_engine(struct intel_engine_cs *engine)
+{
+ struct drm_i915_private *dev_priv = engine->i915;
+ struct intel_guc *guc = &dev_priv->guc;
+ u32 data[7];
+
+ GEM_BUG_ON(!guc->execbuf_client);
+
+ data[0] = INTEL_GUC_ACTION_REQUEST_ENGINE_RESET;
+ data[1] = engine->guc_id;
+ data[2] = 0;
+ data[3] = 0;
+ data[4] = 0;
+ data[5] = guc->execbuf_client->stage_id;
+ data[6] = guc_ggtt_offset(guc->shared_data);
+
+ return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
/**
* intel_guc_resume() - notify GuC resuming from suspend state
* @dev_priv: i915 device private
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index e24dbec2ead8..6a10aa6f04d3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -574,6 +574,7 @@ struct guc_shared_ctx_data {
enum intel_guc_action {
INTEL_GUC_ACTION_DEFAULT = 0x0,
INTEL_GUC_ACTION_REQUEST_PREEMPTION = 0x2,
+ INTEL_GUC_ACTION_REQUEST_ENGINE_RESET = 0x3,
INTEL_GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
INTEL_GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
INTEL_GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index d629b2e40013..0564d87ad416 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1792,14 +1792,9 @@ bool intel_has_gpu_reset(struct drm_i915_private *dev_priv)
return intel_get_gpu_reset(dev_priv) != NULL;
}
-/*
- * When GuC submission is enabled, GuC manages ELSP and can initiate the
- * engine reset too. For now, fall back to full GPU reset if it is enabled.
- */
bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
{
return (dev_priv->info.has_reset_engine &&
- !dev_priv->guc.execbuf_client &&
i915_modparams.reset >= 2);
}
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-30 18:56 ` [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands Michel Thierry
@ 2017-10-30 20:58 ` Chris Wilson
2017-10-30 21:08 ` Michel Thierry
2017-10-30 21:09 ` Chris Wilson
2017-10-31 22:53 ` [PATCH v6] " Michel Thierry
2 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2017-10-30 20:58 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
Quoting Michel Thierry (2017-10-30 18:56:15)
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index af745749509c..02fb35744f66 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1984,10 +1984,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
> goto out;
> }
>
> - ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
> + if (!engine->i915->guc.execbuf_client)
> + ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
> + else
> + ret = intel_guc_reset_engine(engine);
While redundant, the interface cries out for
intel_guc_reset_engine(guc, engine);
even though, in this case we would be using
intel_guc_reset_engine(&engine->i915->guc, engine);
I would also be tempted to change intel_gpu_reset to
intel_gpu_reset_engine(engine->i915, engine);
intel_gt_reset_engine?
Just trying to make our language consistent along each path.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-30 20:58 ` Chris Wilson
@ 2017-10-30 21:08 ` Michel Thierry
0 siblings, 0 replies; 22+ messages in thread
From: Michel Thierry @ 2017-10-30 21:08 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 30/10/17 13:58, Chris Wilson wrote:
> Quoting Michel Thierry (2017-10-30 18:56:15)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index af745749509c..02fb35744f66 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1984,10 +1984,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
>> goto out;
>> }
>>
>> - ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
>> + if (!engine->i915->guc.execbuf_client)
>> + ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
>> + else
>> + ret = intel_guc_reset_engine(engine);
>
> While redundant, the interface cries out for
> intel_guc_reset_engine(guc, engine);
> even though, in this case we would be using
> intel_guc_reset_engine(&engine->i915->guc, engine);
>
I'll change the name and pass the guc as parameter.
> I would also be tempted to change intel_gpu_reset to
> intel_gpu_reset_engine(engine->i915, engine);
>
> intel_gt_reset_engine?
>
And then it becomes a wrapper of intel_gpu_reset? (intel_gpu_reset is
used for full reset path and in older platforms too).
> Just trying to make our language consistent along each path.
>
It's true that it would look better as you suggest:
if (!guc_client)
intel_gt_reset_engine(i915, engine);
else
intel_guc_reset_engine(guc, engine);
Thanks,
-Michel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-30 18:56 ` [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands Michel Thierry
2017-10-30 20:58 ` Chris Wilson
@ 2017-10-30 21:09 ` Chris Wilson
2017-10-31 4:38 ` Michel Thierry
2017-10-31 22:53 ` [PATCH v6] " Michel Thierry
2 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2017-10-30 21:09 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
Quoting Michel Thierry (2017-10-30 18:56:15)
> This patch adds per engine reset and recovery (TDR) support when GuC is
> used to submit workloads to GPU.
>
> In the case of i915 directly submission to ELSP, driver manages hang
> detection, recovery and resubmission. With GuC submission these tasks
> are shared between driver and GuC. i915 is still responsible for detecting
> a hang, and when it does it only requests GuC to reset that Engine. GuC
> internally manages acquiring forcewake and idling the engine before actually
> resetting it.
>
> Once the reset is successful, i915 takes over again and handles resubmission.
> The scheduler in i915 knows which requests are pending so after resetting
> a engine, pending workloads/requests are resubmitted again.
>
> v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
> non-guc funtion names.
>
> v3: Removed debug message about engine restarting from which request,
> since the new baseline do it regardless of submission mode. (Chris)
>
> v4: Rebase.
>
> v5: Do not pass unnecessary reporting flags to the fw (Jeff);
> tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
In your experience, how did our test coverage fare?
Could you use live_hangcheck effectively? (The drv_selftest would need
some hand holding to pass along guc options. But for livetesting we
should probably get to the point of being able to load/unload the guc
interface so that we cover both execlists and guc.) Did you find
"gem_exec_whisper --r hang*", did you try gem_concurrent_all?
> +/**
> + * intel_guc_reset_engine() - ask GuC to reset an engine
> + * @engine: engine to be reset
> + */
> +int intel_guc_reset_engine(struct intel_engine_cs *engine)
> +{
> + struct drm_i915_private *dev_priv = engine->i915;
> + struct intel_guc *guc = &dev_priv->guc;
> + u32 data[7];
> +
> + GEM_BUG_ON(!guc->execbuf_client);
> +
> + data[0] = INTEL_GUC_ACTION_REQUEST_ENGINE_RESET;
> + data[1] = engine->guc_id;
> + data[2] = 0;
> + data[3] = 0;
> + data[4] = 0;
> + data[5] = guc->execbuf_client->stage_id;
> + data[6] = guc_ggtt_offset(guc->shared_data);
> +
> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
Is this a synchronous action? We expect that following the completion of
the reset routine, we are ready to reinit the hw. The same rule needs to
apply the guc, I think.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-30 21:09 ` Chris Wilson
@ 2017-10-31 4:38 ` Michel Thierry
2017-10-31 10:17 ` Chris Wilson
0 siblings, 1 reply; 22+ messages in thread
From: Michel Thierry @ 2017-10-31 4:38 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 30/10/17 14:09, Chris Wilson wrote:
> Quoting Michel Thierry (2017-10-30 18:56:15)
>> This patch adds per engine reset and recovery (TDR) support when GuC is
>> used to submit workloads to GPU.
>>
>> In the case of i915 directly submission to ELSP, driver manages hang
>> detection, recovery and resubmission. With GuC submission these tasks
>> are shared between driver and GuC. i915 is still responsible for detecting
>> a hang, and when it does it only requests GuC to reset that Engine. GuC
>> internally manages acquiring forcewake and idling the engine before actually
>> resetting it.
>>
>> Once the reset is successful, i915 takes over again and handles resubmission.
>> The scheduler in i915 knows which requests are pending so after resetting
>> a engine, pending workloads/requests are resubmitted again.
>>
>> v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
>> non-guc funtion names.
>>
>> v3: Removed debug message about engine restarting from which request,
>> since the new baseline do it regardless of submission mode. (Chris)
>>
>> v4: Rebase.
>>
>> v5: Do not pass unnecessary reporting flags to the fw (Jeff);
>> tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
>
> In your experience, how did our test coverage fare?
>
> Could you use live_hangcheck effectively? (The drv_selftest would need
> some hand holding to pass along guc options. But for livetesting we
> should probably get to the point of being able to load/unload the guc
> interface so that we cover both execlists and guc.) Did you find
> "gem_exec_whisper --r hang*", did you try gem_concurrent_all?
>
live_hangcheck runs ok with guc (as long as i915_params.h has guc
submission enabled). Do you see a benefit on adding an option in
drv_selftest to override the submission mode? I can add it to my list.
You got me in gem_concurrent_all, I forgot to schedule it a few weeks ago.
>> +/**
>> + * intel_guc_reset_engine() - ask GuC to reset an engine
>> + * @engine: engine to be reset
>> + */
>> +int intel_guc_reset_engine(struct intel_engine_cs *engine)
>> +{
>> + struct drm_i915_private *dev_priv = engine->i915;
>> + struct intel_guc *guc = &dev_priv->guc;
>> + u32 data[7];
>> +
>> + GEM_BUG_ON(!guc->execbuf_client);
>> +
>> + data[0] = INTEL_GUC_ACTION_REQUEST_ENGINE_RESET;
>> + data[1] = engine->guc_id;
>> + data[2] = 0;
>> + data[3] = 0;
>> + data[4] = 0;
>> + data[5] = guc->execbuf_client->stage_id;
>> + data[6] = guc_ggtt_offset(guc->shared_data);
>> +
>> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
>
> Is this a synchronous action? We expect that following the completion of
> the reset routine, we are ready to reinit the hw. The same rule needs to
> apply the guc, I think.
Right now the action is synchronous, the fw won't reply to the action
until all the steps are completed. It also is fast enough, I haven't
seen it time out (which would be promoted to full reset and reload the
fw). But, do you have a crystal ball?
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-31 4:38 ` Michel Thierry
@ 2017-10-31 10:17 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2017-10-31 10:17 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
Quoting Michel Thierry (2017-10-31 04:38:30)
> On 30/10/17 14:09, Chris Wilson wrote:
> > Quoting Michel Thierry (2017-10-30 18:56:15)
> >> This patch adds per engine reset and recovery (TDR) support when GuC is
> >> used to submit workloads to GPU.
> >>
> >> In the case of i915 directly submission to ELSP, driver manages hang
> >> detection, recovery and resubmission. With GuC submission these tasks
> >> are shared between driver and GuC. i915 is still responsible for detecting
> >> a hang, and when it does it only requests GuC to reset that Engine. GuC
> >> internally manages acquiring forcewake and idling the engine before actually
> >> resetting it.
> >>
> >> Once the reset is successful, i915 takes over again and handles resubmission.
> >> The scheduler in i915 knows which requests are pending so after resetting
> >> a engine, pending workloads/requests are resubmitted again.
> >>
> >> v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
> >> non-guc funtion names.
> >>
> >> v3: Removed debug message about engine restarting from which request,
> >> since the new baseline do it regardless of submission mode. (Chris)
> >>
> >> v4: Rebase.
> >>
> >> v5: Do not pass unnecessary reporting flags to the fw (Jeff);
> >> tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
> >
> > In your experience, how did our test coverage fare?
> >
> > Could you use live_hangcheck effectively? (The drv_selftest would need
> > some hand holding to pass along guc options. But for livetesting we
> > should probably get to the point of being able to load/unload the guc
> > interface so that we cover both execlists and guc.) Did you find
> > "gem_exec_whisper --r hang*", did you try gem_concurrent_all?
> >
>
> live_hangcheck runs ok with guc (as long as i915_params.h has guc
> submission enabled). Do you see a benefit on adding an option in
> drv_selftest to override the submission mode? I can add it to my list.
I'd rather take the path of loading/unloading guc in the kernel to
ensure we have coverage of both on all platforms where that makes sense.
That way the kernel remains in control of the coverage it deems
necessary.
The alternative would be a for_each_param_value() loop in drv_selftests
which doesn't inspired me with forward/backward testing confidence.
For the time being, knowing that you did check live_hangcheck is enough
for me to cross it off the checklist.
The larger question behind "how did our test coverage fare" is did it
find any bugs during development? If not, we need more tests ;)
> You got me in gem_concurrent_all, I forgot to schedule it a few weeks ago.
>
> >> +/**
> >> + * intel_guc_reset_engine() - ask GuC to reset an engine
> >> + * @engine: engine to be reset
> >> + */
> >> +int intel_guc_reset_engine(struct intel_engine_cs *engine)
> >> +{
> >> + struct drm_i915_private *dev_priv = engine->i915;
> >> + struct intel_guc *guc = &dev_priv->guc;
> >> + u32 data[7];
> >> +
> >> + GEM_BUG_ON(!guc->execbuf_client);
> >> +
> >> + data[0] = INTEL_GUC_ACTION_REQUEST_ENGINE_RESET;
> >> + data[1] = engine->guc_id;
> >> + data[2] = 0;
> >> + data[3] = 0;
> >> + data[4] = 0;
> >> + data[5] = guc->execbuf_client->stage_id;
> >> + data[6] = guc_ggtt_offset(guc->shared_data);
> >> +
> >> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
> >
> > Is this a synchronous action? We expect that following the completion of
> > the reset routine, we are ready to reinit the hw. The same rule needs to
> > apply the guc, I think.
>
> Right now the action is synchronous, the fw won't reply to the action
> until all the steps are completed. It also is fast enough, I haven't
> seen it time out (which would be promoted to full reset and reload the
> fw). But, do you have a crystal ball?
Just REQUEST is a red flag that there may be an asynchronous REPLY/ACK.
Aside: we need to start planning/adding fault_injection tests.
intel_guc_send() seems a good choice for an if(should_fail()).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v6] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-30 18:56 ` [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands Michel Thierry
2017-10-30 20:58 ` Chris Wilson
2017-10-30 21:09 ` Chris Wilson
@ 2017-10-31 22:53 ` Michel Thierry
2017-11-01 13:58 ` Chris Wilson
2 siblings, 1 reply; 22+ messages in thread
From: Michel Thierry @ 2017-10-31 22:53 UTC (permalink / raw)
To: intel-gfx
This patch adds per engine reset and recovery (TDR) support when GuC is
used to submit workloads to GPU.
In the case of i915 directly submission to ELSP, driver manages hang
detection, recovery and resubmission. With GuC submission these tasks
are shared between driver and GuC. i915 is still responsible for detecting
a hang, and when it does it only requests GuC to reset that Engine. GuC
internally manages acquiring forcewake and idling the engine before
resetting it.
Once the reset is successful, i915 takes over again and handles the
resubmission. The scheduler in i915 knows which requests are pending so
after resetting a engine, pending workloads/requests are resubmitted
again.
v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
non-guc function names.
v3: Removed debug message about engine restarting from which request,
since the new baseline do it regardless of submission mode. (Chris)
v4: Rebase.
v5: Do not pass unnecessary reporting flags to the fw (Jeff);
tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
v6: Rename the existing reset engine function and share a similar
interface between guc and non-guc paths (Chris).
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++--
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_guc.c | 24 ++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_guc_fwif.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 5 -----
5 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index af745749509c..359333a423cf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1950,6 +1950,12 @@ void i915_reset(struct drm_i915_private *i915, unsigned int flags)
goto finish;
}
+static inline int intel_gt_reset_engine(struct drm_i915_private *dev_priv,
+ struct intel_engine_cs *engine)
+{
+ return intel_gpu_reset(dev_priv, intel_engine_flag(engine));
+}
+
/**
* i915_reset_engine - reset GPU engine to recover from a hang
* @engine: engine to reset
@@ -1984,10 +1990,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
goto out;
}
- ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
+ if (!engine->i915->guc.execbuf_client)
+ ret = intel_gt_reset_engine(engine->i915, engine);
+ else
+ ret = intel_guc_reset_engine(&engine->i915->guc, engine);
+
if (ret) {
/* If we fail here, we expect to fallback to a global reset */
- DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n",
+ DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
+ (engine->i915->guc.execbuf_client ? "GUC ":""),
engine->name, ret);
goto out;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cff1b57598c3..ce2725696187 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3330,6 +3330,8 @@ extern int i915_reset_engine(struct intel_engine_cs *engine,
extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv);
extern int intel_reset_guc(struct drm_i915_private *dev_priv);
+extern int intel_guc_reset_engine(struct intel_guc *guc,
+ struct intel_engine_cs *engine);
extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
extern void intel_hangcheck_init(struct drm_i915_private *dev_priv);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index f74d50fdaeb0..9678630a1c70 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -24,6 +24,7 @@
#include "intel_guc.h"
#include "i915_drv.h"
+#include "i915_guc_submission.h"
static void gen8_guc_raise_irq(struct intel_guc *guc)
{
@@ -283,6 +284,29 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
+/**
+ * intel_guc_reset_engine() - ask GuC to reset an engine
+ * @guc: intel_guc structure
+ * @engine: engine to be reset
+ */
+int intel_guc_reset_engine(struct intel_guc *guc,
+ struct intel_engine_cs *engine)
+{
+ u32 data[7];
+
+ GEM_BUG_ON(!guc->execbuf_client);
+
+ data[0] = INTEL_GUC_ACTION_REQUEST_ENGINE_RESET;
+ data[1] = engine->guc_id;
+ data[2] = 0;
+ data[3] = 0;
+ data[4] = 0;
+ data[5] = guc->execbuf_client->stage_id;
+ data[6] = guc_ggtt_offset(guc->shared_data);
+
+ return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
/**
* intel_guc_resume() - notify GuC resuming from suspend state
* @dev_priv: i915 device private
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index e24dbec2ead8..6a10aa6f04d3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -574,6 +574,7 @@ struct guc_shared_ctx_data {
enum intel_guc_action {
INTEL_GUC_ACTION_DEFAULT = 0x0,
INTEL_GUC_ACTION_REQUEST_PREEMPTION = 0x2,
+ INTEL_GUC_ACTION_REQUEST_ENGINE_RESET = 0x3,
INTEL_GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
INTEL_GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
INTEL_GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index d629b2e40013..0564d87ad416 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1792,14 +1792,9 @@ bool intel_has_gpu_reset(struct drm_i915_private *dev_priv)
return intel_get_gpu_reset(dev_priv) != NULL;
}
-/*
- * When GuC submission is enabled, GuC manages ELSP and can initiate the
- * engine reset too. For now, fall back to full GPU reset if it is enabled.
- */
bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
{
return (dev_priv->info.has_reset_engine &&
- !dev_priv->guc.execbuf_client &&
i915_modparams.reset >= 2);
}
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v6] drm/i915/guc: Add support for reset engine using GuC commands
2017-10-31 22:53 ` [PATCH v6] " Michel Thierry
@ 2017-11-01 13:58 ` Chris Wilson
2017-11-01 20:41 ` Jeff McGee
0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2017-11-01 13:58 UTC (permalink / raw)
To: Michel Thierry, intel-gfx
Quoting Michel Thierry (2017-10-31 22:53:09)
> This patch adds per engine reset and recovery (TDR) support when GuC is
> used to submit workloads to GPU.
>
> In the case of i915 directly submission to ELSP, driver manages hang
> detection, recovery and resubmission. With GuC submission these tasks
> are shared between driver and GuC. i915 is still responsible for detecting
> a hang, and when it does it only requests GuC to reset that Engine. GuC
> internally manages acquiring forcewake and idling the engine before
> resetting it.
>
> Once the reset is successful, i915 takes over again and handles the
> resubmission. The scheduler in i915 knows which requests are pending so
> after resetting a engine, pending workloads/requests are resubmitted
> again.
>
> v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
> non-guc function names.
>
> v3: Removed debug message about engine restarting from which request,
> since the new baseline do it regardless of submission mode. (Chris)
>
> v4: Rebase.
>
> v5: Do not pass unnecessary reporting flags to the fw (Jeff);
> tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
>
> v6: Rename the existing reset engine function and share a similar
> interface between guc and non-guc paths (Chris).
>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++--
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_guc.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc_fwif.h | 1 +
> drivers/gpu/drm/i915/intel_uncore.c | 5 -----
> 5 files changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index af745749509c..359333a423cf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1950,6 +1950,12 @@ void i915_reset(struct drm_i915_private *i915, unsigned int flags)
> goto finish;
> }
>
> +static inline int intel_gt_reset_engine(struct drm_i915_private *dev_priv,
> + struct intel_engine_cs *engine)
> +{
> + return intel_gpu_reset(dev_priv, intel_engine_flag(engine));
> +}
> +
> /**
> * i915_reset_engine - reset GPU engine to recover from a hang
> * @engine: engine to reset
> @@ -1984,10 +1990,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
> goto out;
> }
>
> - ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
> + if (!engine->i915->guc.execbuf_client)
> + ret = intel_gt_reset_engine(engine->i915, engine);
> + else
> + ret = intel_guc_reset_engine(&engine->i915->guc, engine);
> +
> if (ret) {
> /* If we fail here, we expect to fallback to a global reset */
> - DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n",
> + DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
> + (engine->i915->guc.execbuf_client ? "GUC ":""),
A bit overkill on the parentheses there ;)
Lgtm, can you please ping, say, Jeff or Daniele for an r-b on the guc
interaction?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v6] drm/i915/guc: Add support for reset engine using GuC commands
2017-11-01 13:58 ` Chris Wilson
@ 2017-11-01 20:41 ` Jeff McGee
2017-11-02 8:43 ` Chris Wilson
0 siblings, 1 reply; 22+ messages in thread
From: Jeff McGee @ 2017-11-01 20:41 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Nov 01, 2017 at 01:58:04PM +0000, Chris Wilson wrote:
> Quoting Michel Thierry (2017-10-31 22:53:09)
> > This patch adds per engine reset and recovery (TDR) support when GuC is
> > used to submit workloads to GPU.
> >
> > In the case of i915 directly submission to ELSP, driver manages hang
> > detection, recovery and resubmission. With GuC submission these tasks
> > are shared between driver and GuC. i915 is still responsible for detecting
> > a hang, and when it does it only requests GuC to reset that Engine. GuC
> > internally manages acquiring forcewake and idling the engine before
> > resetting it.
> >
> > Once the reset is successful, i915 takes over again and handles the
> > resubmission. The scheduler in i915 knows which requests are pending so
> > after resetting a engine, pending workloads/requests are resubmitted
> > again.
> >
> > v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
> > non-guc function names.
> >
> > v3: Removed debug message about engine restarting from which request,
> > since the new baseline do it regardless of submission mode. (Chris)
> >
> > v4: Rebase.
> >
> > v5: Do not pass unnecessary reporting flags to the fw (Jeff);
> > tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
> >
> > v6: Rename the existing reset engine function and share a similar
> > interface between guc and non-guc paths (Chris).
> >
> > Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++--
> > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_guc.c | 24 ++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 +
> > drivers/gpu/drm/i915/intel_uncore.c | 5 -----
> > 5 files changed, 40 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index af745749509c..359333a423cf 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1950,6 +1950,12 @@ void i915_reset(struct drm_i915_private *i915, unsigned int flags)
> > goto finish;
> > }
> >
> > +static inline int intel_gt_reset_engine(struct drm_i915_private *dev_priv,
> > + struct intel_engine_cs *engine)
> > +{
> > + return intel_gpu_reset(dev_priv, intel_engine_flag(engine));
> > +}
> > +
> > /**
> > * i915_reset_engine - reset GPU engine to recover from a hang
> > * @engine: engine to reset
> > @@ -1984,10 +1990,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
> > goto out;
> > }
> >
> > - ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
> > + if (!engine->i915->guc.execbuf_client)
> > + ret = intel_gt_reset_engine(engine->i915, engine);
> > + else
> > + ret = intel_guc_reset_engine(&engine->i915->guc, engine);
> > +
> > if (ret) {
> > /* If we fail here, we expect to fallback to a global reset */
> > - DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n",
> > + DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
> > + (engine->i915->guc.execbuf_client ? "GUC ":""),
>
> A bit overkill on the parentheses there ;)
>
> Lgtm, can you please ping, say, Jeff or Daniele for an r-b on the guc
> interaction?
> -Chris
There is one small change needed in the GuC preemption protocol to make it
compatible with GuC engine reset. I will send that shortly.
There are also a couple of corner case bugs with engine reset in our current
firmware versions. We are planning a firmware update to address those. But
the host-side code here is fine. So...
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v6] drm/i915/guc: Add support for reset engine using GuC commands
2017-11-01 20:41 ` Jeff McGee
@ 2017-11-02 8:43 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2017-11-02 8:43 UTC (permalink / raw)
To: Jeff McGee; +Cc: intel-gfx
Quoting Jeff McGee (2017-11-01 20:41:13)
> On Wed, Nov 01, 2017 at 01:58:04PM +0000, Chris Wilson wrote:
> > Quoting Michel Thierry (2017-10-31 22:53:09)
> > > This patch adds per engine reset and recovery (TDR) support when GuC is
> > > used to submit workloads to GPU.
> > >
> > > In the case of i915 directly submission to ELSP, driver manages hang
> > > detection, recovery and resubmission. With GuC submission these tasks
> > > are shared between driver and GuC. i915 is still responsible for detecting
> > > a hang, and when it does it only requests GuC to reset that Engine. GuC
> > > internally manages acquiring forcewake and idling the engine before
> > > resetting it.
> > >
> > > Once the reset is successful, i915 takes over again and handles the
> > > resubmission. The scheduler in i915 knows which requests are pending so
> > > after resetting a engine, pending workloads/requests are resubmitted
> > > again.
> > >
> > > v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
> > > non-guc function names.
> > >
> > > v3: Removed debug message about engine restarting from which request,
> > > since the new baseline do it regardless of submission mode. (Chris)
> > >
> > > v4: Rebase.
> > >
> > > v5: Do not pass unnecessary reporting flags to the fw (Jeff);
> > > tasklet_schedule(&execlists->irq_tasklet) handles the resubmit; rebase.
> > >
> > > v6: Rename the existing reset engine function and share a similar
> > > interface between guc and non-guc paths (Chris).
> > >
> > > Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++--
> > > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > > drivers/gpu/drm/i915/intel_guc.c | 24 ++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 +
> > > drivers/gpu/drm/i915/intel_uncore.c | 5 -----
> > > 5 files changed, 40 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index af745749509c..359333a423cf 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1950,6 +1950,12 @@ void i915_reset(struct drm_i915_private *i915, unsigned int flags)
> > > goto finish;
> > > }
> > >
> > > +static inline int intel_gt_reset_engine(struct drm_i915_private *dev_priv,
> > > + struct intel_engine_cs *engine)
> > > +{
> > > + return intel_gpu_reset(dev_priv, intel_engine_flag(engine));
> > > +}
> > > +
> > > /**
> > > * i915_reset_engine - reset GPU engine to recover from a hang
> > > * @engine: engine to reset
> > > @@ -1984,10 +1990,15 @@ int i915_reset_engine(struct intel_engine_cs *engine, unsigned int flags)
> > > goto out;
> > > }
> > >
> > > - ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine));
> > > + if (!engine->i915->guc.execbuf_client)
> > > + ret = intel_gt_reset_engine(engine->i915, engine);
> > > + else
> > > + ret = intel_guc_reset_engine(&engine->i915->guc, engine);
> > > +
> > > if (ret) {
> > > /* If we fail here, we expect to fallback to a global reset */
> > > - DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n",
> > > + DRM_DEBUG_DRIVER("%sFailed to reset %s, ret=%d\n",
> > > + (engine->i915->guc.execbuf_client ? "GUC ":""),
> >
> > A bit overkill on the parentheses there ;)
> >
> > Lgtm, can you please ping, say, Jeff or Daniele for an r-b on the guc
> > interaction?
> > -Chris
>
> There is one small change needed in the GuC preemption protocol to make it
> compatible with GuC engine reset. I will send that shortly.
>
> There are also a couple of corner case bugs with engine reset in our current
> firmware versions. We are planning a firmware update to address those. But
> the host-side code here is fine. So...
>
> Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
Pushed this series, along with the preempt interaction fix.
Thanks,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] HAX enable GuC submission for CI
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
2017-10-30 18:56 ` [PATCH 1/3] drm/i915/guc: Rename the function that resets the GuC Michel Thierry
2017-10-30 18:56 ` [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands Michel Thierry
@ 2017-10-30 18:56 ` Michel Thierry
2017-10-30 20:05 ` ✓ Fi.CI.BAT: success for GuC based reset engine Patchwork
` (3 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Michel Thierry @ 2017-10-30 18:56 UTC (permalink / raw)
To: intel-gfx
From: Michal Wajdeczko <michal.wajdeczko@intel.com>
Also revert ("drm/i915/guc: Assert that we switch between
known ggtt->invalidate functions")
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
drivers/gpu/drm/i915/i915_params.h | 4 ++--
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5eaa6893daaa..7dcf931abe37 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3568,17 +3568,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
void i915_ggtt_enable_guc(struct drm_i915_private *i915)
{
- GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
i915->ggtt.invalidate = guc_ggtt_invalidate;
}
void i915_ggtt_disable_guc(struct drm_i915_private *i915)
{
- /* We should only be called after i915_ggtt_enable_guc() */
- GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
- i915->ggtt.invalidate = gen6_ggtt_invalidate;
+ if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+ i915->ggtt.invalidate = gen6_ggtt_invalidate;
}
void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c7292268ed43..c38cef07b9fe 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,8 +44,8 @@
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc_loading, 0) \
- param(int, enable_guc_submission, 0) \
+ param(int, enable_guc_loading, 1) \
+ param(int, enable_guc_submission, 1) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
2.14.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread* ✓ Fi.CI.BAT: success for GuC based reset engine
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
` (2 preceding siblings ...)
2017-10-30 18:56 ` [PATCH 3/3] HAX enable GuC submission for CI Michel Thierry
@ 2017-10-30 20:05 ` Patchwork
2017-10-30 21:14 ` Chris Wilson
2017-10-30 23:20 ` ✗ Fi.CI.IGT: warning " Patchwork
` (2 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Patchwork @ 2017-10-30 20:05 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
== Series Details ==
Series: GuC based reset engine
URL : https://patchwork.freedesktop.org/series/32859/
State : success
== Summary ==
Series 32859v1 GuC based reset engine
https://patchwork.freedesktop.org/api/1.0/series/32859/revisions/1/mbox/
Test chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#102514
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-cfl-s) fdo#103186
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
incomplete -> PASS (fi-bxt-dsi)
Test pm_rpm:
Subgroup basic-pci-d3-state:
incomplete -> FAIL (fi-bxt-j4205)
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#103186 https://bugs.freedesktop.org/show_bug.cgi?id=103186
fi-bdw-5557u total:289 pass:266 dwarn:0 dfail:0 fail:2 skip:21 time:459s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:454s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:371s
fi-bsw-n3050 total:289 pass:241 dwarn:0 dfail:0 fail:2 skip:46 time:540s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:265s
fi-bxt-dsi total:289 pass:257 dwarn:0 dfail:0 fail:2 skip:30 time:517s
fi-bxt-j4205 total:289 pass:258 dwarn:0 dfail:0 fail:2 skip:29 time:520s
fi-byt-j1900 total:289 pass:251 dwarn:1 dfail:0 fail:2 skip:35 time:510s
fi-byt-n2820 total:289 pass:247 dwarn:1 dfail:0 fail:2 skip:39 time:499s
fi-cfl-s total:289 pass:251 dwarn:4 dfail:0 fail:2 skip:32 time:571s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:423s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:249s
fi-glk-1 total:289 pass:259 dwarn:0 dfail:0 fail:2 skip:28 time:592s
fi-hsw-4770 total:289 pass:260 dwarn:0 dfail:0 fail:2 skip:27 time:444s
fi-hsw-4770r total:289 pass:260 dwarn:0 dfail:0 fail:2 skip:27 time:442s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:421s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:488s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:458s
fi-kbl-7500u total:289 pass:261 dwarn:1 dfail:0 fail:3 skip:24 time:500s
fi-kbl-7560u total:289 pass:268 dwarn:0 dfail:0 fail:2 skip:19 time:582s
fi-kbl-7567u total:289 pass:267 dwarn:0 dfail:0 fail:2 skip:20 time:489s
fi-kbl-r total:289 pass:260 dwarn:0 dfail:0 fail:2 skip:27 time:588s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:537s
fi-skl-6260u total:289 pass:267 dwarn:0 dfail:0 fail:2 skip:20 time:474s
fi-skl-6600u total:289 pass:260 dwarn:0 dfail:0 fail:2 skip:27 time:593s
fi-skl-6700hq total:289 pass:261 dwarn:0 dfail:0 fail:2 skip:26 time:652s
fi-skl-6700k total:289 pass:263 dwarn:0 dfail:0 fail:2 skip:24 time:537s
fi-skl-6770hq total:289 pass:267 dwarn:0 dfail:0 fail:2 skip:20 time:515s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:459s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:554s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:415s
302b1723bec5e7f43e423c975b9b87705fb9fdd6 drm-tip: 2017y-10m-30d-18h-28m-39s UTC integration manifest
e25d1ee5b093 HAX enable GuC submission for CI
c3a835c2c17c drm/i915/guc: Add support for reset engine using GuC commands
195f89fa1970 drm/i915/guc: Rename the function that resets the GuC
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6267/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: ✓ Fi.CI.BAT: success for GuC based reset engine
2017-10-30 20:05 ` ✓ Fi.CI.BAT: success for GuC based reset engine Patchwork
@ 2017-10-30 21:14 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2017-10-30 21:14 UTC (permalink / raw)
To: Patchwork, Michel Thierry; +Cc: intel-gfx
Quoting Patchwork (2017-10-30 20:05:05)
> == Series Details ==
>
> Series: GuC based reset engine
> URL : https://patchwork.freedesktop.org/series/32859/
> State : success
>
> == Summary ==
>
> Series 32859v1 GuC based reset engine
> https://patchwork.freedesktop.org/api/1.0/series/32859/revisions/1/mbox/
>
> Test chamelium:
> Subgroup dp-crc-fast:
> pass -> FAIL (fi-kbl-7500u) fdo#102514
> Test gem_exec_suspend:
> Subgroup basic-s3:
> pass -> DMESG-WARN (fi-cfl-s) fdo#103186
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-c:
> incomplete -> PASS (fi-bxt-dsi)
> Test pm_rpm:
> Subgroup basic-pci-d3-state:
> incomplete -> FAIL (fi-bxt-j4205)
35686a44e4b8 drm/i915: Use intel_ddi_get_config() for MST
1939ba51fd05 drm/i915: Pass a crtc state to ddi post_disable from MST code
bb911536f07e drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
0fce04c8764b drm/i915: Nuke intel_ddi_get_encoder_port()
7e732cacb1ae drm/i915: Stop frobbing with DDI encoder->type
e1214b95ed83 drm/i915: Populate output_types from .get_config()
d6038611aa3d drm/i915: Parse max HDMI TMDS clock from VBT
6e8fbf8d19e4 drm/i915/vbt: Fix HDMI level shifter and max data rate bitfield sizes
2dd14a4ad87f drm-tip: 2017y-10m-30d-13h-40m-22s UTC integration manifest
0b07194bb55e Linux 4.14-rc7
From the rc7 patchset, my guess would be
0cc2b4e5a020 PM / QoS: Fix device resume latency PM QoS
Anyway, someone has a fun bisect task on their hang; it should be very
quick.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* ✗ Fi.CI.IGT: warning for GuC based reset engine
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
` (3 preceding siblings ...)
2017-10-30 20:05 ` ✓ Fi.CI.BAT: success for GuC based reset engine Patchwork
@ 2017-10-30 23:20 ` Patchwork
2017-10-31 10:20 ` Chris Wilson
2017-10-31 23:50 ` ✓ Fi.CI.BAT: success for GuC based reset engine (rev2) Patchwork
2017-11-01 0:59 ` ✗ Fi.CI.IGT: failure " Patchwork
6 siblings, 1 reply; 22+ messages in thread
From: Patchwork @ 2017-10-30 23:20 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
== Series Details ==
Series: GuC based reset engine
URL : https://patchwork.freedesktop.org/series/32859/
State : warning
== Summary ==
Test drv_module_reload:
Subgroup basic-no-display:
pass -> DMESG-WARN (shard-hsw) fdo#102707
Test gem_softpin:
Subgroup noreloc-S3:
pass -> SKIP (shard-hsw)
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
shard-hsw total:2539 pass:1390 dwarn:2 dfail:0 fail:52 skip:1095 time:9075s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6267/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* ✓ Fi.CI.BAT: success for GuC based reset engine (rev2)
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
` (4 preceding siblings ...)
2017-10-30 23:20 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2017-10-31 23:50 ` Patchwork
2017-11-01 0:59 ` ✗ Fi.CI.IGT: failure " Patchwork
6 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2017-10-31 23:50 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
== Series Details ==
Series: GuC based reset engine (rev2)
URL : https://patchwork.freedesktop.org/series/32859/
State : success
== Summary ==
Series 32859v2 GuC based reset engine
https://patchwork.freedesktop.org/api/1.0/series/32859/revisions/2/mbox/
Test chamelium:
Subgroup dp-crc-fast:
fail -> PASS (fi-kbl-7500u) fdo#102514
Test drv_module_reload:
Subgroup basic-no-display:
fail -> PASS (fi-hsw-4770r)
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:450s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:453s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:553s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:276s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:515s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:507s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:513s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:492s
fi-cfl-s total:289 pass:253 dwarn:4 dfail:0 fail:0 skip:32 time:557s
fi-cnl-y total:217 pass:196 dwarn:0 dfail:0 fail:0 skip:20
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:435s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:265s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:587s
fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:495s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:435s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:426s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:425s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:506s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:465s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:489s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:574s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:472s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:584s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:569s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:455s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:591s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:653s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:521s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:496s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:463s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:567s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:421s
98f067e087ab9294555a9bdd6a056c4156777117 drm-tip: 2017y-10m-31d-20h-56m-13s UTC integration manifest
2fda8dacaf77 HAX enable GuC submission for CI
2f3b79915935 drm/i915/guc: Add support for reset engine using GuC commands
614ac0cbdb82 drm/i915/guc: Rename the function that resets the GuC
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6284/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* ✗ Fi.CI.IGT: failure for GuC based reset engine (rev2)
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
` (5 preceding siblings ...)
2017-10-31 23:50 ` ✓ Fi.CI.BAT: success for GuC based reset engine (rev2) Patchwork
@ 2017-11-01 0:59 ` Patchwork
6 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2017-11-01 0:59 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
== Series Details ==
Series: GuC based reset engine (rev2)
URL : https://patchwork.freedesktop.org/series/32859/
State : failure
== Summary ==
Test drv_module_reload:
Subgroup basic-reload-inject:
pass -> DMESG-WARN (shard-hsw) fdo#102707
Test kms_sysfs_edid_timing:
pass -> FAIL (shard-hsw) fdo#100047
Test kms_flip:
Subgroup flip-vs-dpms-interruptible:
pass -> INCOMPLETE (shard-hsw)
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
shard-hsw total:2536 pass:1426 dwarn:2 dfail:0 fail:10 skip:1097 time:9009s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6284/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread