* [PATCH] drm/i915: Park before resetting the submission backend
@ 2018-04-09 10:18 Chris Wilson
2018-04-09 10:38 ` Sagar Arun Kamble
2018-04-09 11:01 ` ✗ Fi.CI.BAT: warning for drm/i915: Park before resetting the submission backend (rev2) Patchwork
0 siblings, 2 replies; 7+ messages in thread
From: Chris Wilson @ 2018-04-09 10:18 UTC (permalink / raw)
To: intel-gfx
As different backends may have different park/unpark callbacks, we
should only ever switch backends (reset_default_submission on wedge
recovery, or on enabling the guc) while parked.
v2: Remove the assert from the guc code, as we are currently trying to
modify the engine vfuncs pointer on a live system after reset (not just
wedging). We will just have to hope that the system is balanced.
v3: Rebase onto __i915_gem_park and improve grammar.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++---
drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 28ab0beff86c..dd3e292ba243 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -144,8 +144,6 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
if (!i915->gt.awake)
return I915_EPOCH_INVALID;
- GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID);
-
/*
* Be paranoid and flush a concurrent interrupt to make sure
* we don't reactivate any irq tasklets after parking.
@@ -173,6 +171,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
intel_runtime_pm_put(i915);
+ GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID);
return i915->gt.epoch;
}
@@ -3435,7 +3434,17 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
}
}
i915_retire_requests(i915);
- GEM_BUG_ON(i915->gt.active_requests);
+
+ /*
+ * Park before disengaging the old submit mechanism as different
+ * backends may have different park/unpack callbacks.
+ *
+ * We are idle; the idle-worker will be queued, but we need to run
+ * it now. As we already hold the struct mutex, we can park the GPU
+ * right away, letting the lazy worker see that we are already active
+ * again by the time it acquires the mutex.
+ */
+ __i915_gem_park(i915);
/*
* Undo nop_submit_request. We prevent all new i915 requests from
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 12486d8f534b..b4ea77a2896c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1651,6 +1651,9 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915)
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ /* Must be parked first! */
+ GEM_BUG_ON(i915->gt.awake);
+
for_each_engine(engine, i915, id)
engine->set_default_submission(engine);
}
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Park before resetting the submission backend
2018-04-09 10:18 [PATCH] drm/i915: Park before resetting the submission backend Chris Wilson
@ 2018-04-09 10:38 ` Sagar Arun Kamble
2018-04-09 10:41 ` Chris Wilson
2018-04-09 11:01 ` ✗ Fi.CI.BAT: warning for drm/i915: Park before resetting the submission backend (rev2) Patchwork
1 sibling, 1 reply; 7+ messages in thread
From: Sagar Arun Kamble @ 2018-04-09 10:38 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 4/9/2018 3:48 PM, Chris Wilson wrote:
> As different backends may have different park/unpark callbacks, we
> should only ever switch backends (reset_default_submission on wedge
> recovery, or on enabling the guc) while parked.
>
> v2: Remove the assert from the guc code, as we are currently trying to
> modify the engine vfuncs pointer on a live system after reset (not just
> wedging). We will just have to hope that the system is balanced.
> v3: Rebase onto __i915_gem_park and improve grammar.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++---
> drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 28ab0beff86c..dd3e292ba243 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -144,8 +144,6 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
> if (!i915->gt.awake)
> return I915_EPOCH_INVALID;
>
> - GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID);
> -
> /*
> * Be paranoid and flush a concurrent interrupt to make sure
> * we don't reactivate any irq tasklets after parking.
> @@ -173,6 +171,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
>
> intel_runtime_pm_put(i915);
>
> + GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID);
> return i915->gt.epoch;
> }
>
> @@ -3435,7 +3434,17 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
> }
> }
> i915_retire_requests(i915);
> - GEM_BUG_ON(i915->gt.active_requests);
> +
> + /*
> + * Park before disengaging the old submit mechanism as different
> + * backends may have different park/unpack callbacks.
> + *
> + * We are idle; the idle-worker will be queued, but we need to run
> + * it now. As we already hold the struct mutex, we can park the GPU
> + * right away, letting the lazy worker see that we are already active
> + * again by the time it acquires the mutex.
> + */
> + __i915_gem_park(i915);
>
> /*
> * Undo nop_submit_request. We prevent all new i915 requests from
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 12486d8f534b..b4ea77a2896c 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1651,6 +1651,9 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915)
> struct intel_engine_cs *engine;
> enum intel_engine_id id;
>
> + /* Must be parked first! */
> + GEM_BUG_ON(i915->gt.awake);
> +
> for_each_engine(engine, i915, id)
> engine->set_default_submission(engine);
> }
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Park before resetting the submission backend
2018-04-09 10:38 ` Sagar Arun Kamble
@ 2018-04-09 10:41 ` Chris Wilson
2018-04-09 10:49 ` Michal Wajdeczko
0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2018-04-09 10:41 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
Quoting Sagar Arun Kamble (2018-04-09 11:38:34)
>
>
> On 4/9/2018 3:48 PM, Chris Wilson wrote:
> > As different backends may have different park/unpark callbacks, we
> > should only ever switch backends (reset_default_submission on wedge
> > recovery, or on enabling the guc) while parked.
> >
> > v2: Remove the assert from the guc code, as we are currently trying to
> > modify the engine vfuncs pointer on a live system after reset (not just
> > wedging). We will just have to hope that the system is balanced.
> > v3: Rebase onto __i915_gem_park and improve grammar.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Michal, do you want to take this and merge it into your series?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Park before resetting the submission backend
2018-04-09 10:41 ` Chris Wilson
@ 2018-04-09 10:49 ` Michal Wajdeczko
0 siblings, 0 replies; 7+ messages in thread
From: Michal Wajdeczko @ 2018-04-09 10:49 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx, Chris Wilson
On Mon, 09 Apr 2018 12:41:22 +0200, Chris Wilson
<chris@chris-wilson.co.uk> wrote:
> Quoting Sagar Arun Kamble (2018-04-09 11:38:34)
>>
>>
>> On 4/9/2018 3:48 PM, Chris Wilson wrote:
>> > As different backends may have different park/unpark callbacks, we
>> > should only ever switch backends (reset_default_submission on wedge
>> > recovery, or on enabling the guc) while parked.
>> >
>> > v2: Remove the assert from the guc code, as we are currently trying to
>> > modify the engine vfuncs pointer on a live system after reset (not
>> just
>> > wedging). We will just have to hope that the system is balanced.
>> > v3: Rebase onto __i915_gem_park and improve grammar.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> Michal, do you want to take this and merge it into your series?
Yes, I can take it.
Then we will see results from CI.IGT with GuC enabled.
/Michal
ps. this patch is also
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Park before resetting the submission backend (rev2)
2018-04-09 10:18 [PATCH] drm/i915: Park before resetting the submission backend Chris Wilson
2018-04-09 10:38 ` Sagar Arun Kamble
@ 2018-04-09 11:01 ` Patchwork
1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-04-09 11:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Park before resetting the submission backend (rev2)
URL : https://patchwork.freedesktop.org/series/41202/
State : warning
== Summary ==
Series 41202v2 drm/i915: Park before resetting the submission backend
https://patchwork.freedesktop.org/api/1.0/series/41202/revisions/2/mbox/
---- Possible new issues:
Test gem_exec_gttfill:
Subgroup basic:
pass -> SKIP (fi-pnv-d510)
---- Known issues:
Test debugfs_test:
Subgroup read_all_entries:
incomplete -> PASS (fi-snb-2520m) fdo#103713 +1
Test gem_ctx_param:
Subgroup basic-default:
incomplete -> PASS (fi-cnl-y3) fdo#105086
Test kms_chamelium:
Subgroup dp-edid-read:
fail -> PASS (fi-kbl-7500u) fdo#102505
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (fi-cfl-s3) fdo#100368
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:434s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:444s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:383s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:543s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:296s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:518s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:505s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:412s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:562s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:590s
fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:426s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:314s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:535s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:405s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:422s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:464s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:438s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:474s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:466s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:507s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:644s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:442s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:532s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:500s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:506s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:426s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:444s
fi-snb-2520m total:242 pass:208 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:408s
1be073153147c5c39cdcbdfdeb4e2595ba595bf7 drm-tip: 2018y-04m-07d-22h-26m-31s UTC integration manifest
03338d1167d7 drm/i915: Park before resetting the submission backend
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8637/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Park before resetting the submission backend
2018-04-05 10:57 [PATCH] drm/i915: Park before resetting the submission backend Chris Wilson
@ 2018-04-05 11:03 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2018-04-05 11:03 UTC (permalink / raw)
To: intel-gfx
Quoting Chris Wilson (2018-04-05 11:57:22)
> As different backends may have different park/unpark callbacks, we
> should only ever switch backends (reset_default_submission on wedge
> recovery, or on enabling the guc) while parked.
Last in series, missing the series... Sorry for the noise,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drm/i915: Park before resetting the submission backend
@ 2018-04-05 10:57 Chris Wilson
2018-04-05 11:03 ` Chris Wilson
0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2018-04-05 10:57 UTC (permalink / raw)
To: intel-gfx
As different backends may have different park/unpark callbacks, we
should only ever switch backends (reset_default_submission on wedge
recovery, or on enabling the guc) while parked.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++
drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
3 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e148db310ea6..e2880de2fc7e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3380,6 +3380,17 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
i915_retire_requests(i915);
GEM_BUG_ON(i915->gt.active_requests);
+ /*
+ * Park before disengaging the old submit mechanism as different
+ * backends may have different park/unpack callbacks.
+ *
+ * We are idle; the idle-worker will be queued, but we need to run
+ * it now. As we already hold the struct mutex, we can get park
+ * the GPU right away, letting the lazy worker see that we are
+ * already active again by the time it acquires the mutex.
+ */
+ i915_gem_park(i915);
+
/*
* Undo nop_submit_request. We prevent all new i915 requests from
* being queued (by disallowing execbuf whilst wedged) so having
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 12486d8f534b..b4ea77a2896c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1651,6 +1651,9 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915)
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ /* Must be parked first! */
+ GEM_BUG_ON(i915->gt.awake);
+
for_each_engine(engine, i915, id)
engine->set_default_submission(engine);
}
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 97121230656c..225fa3927a02 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -1243,6 +1243,7 @@ int intel_guc_submission_enable(struct intel_guc *guc)
/* Take over from manual control of ELSP (execlists) */
guc_interrupts_capture(dev_priv);
+ GEM_BUG_ON(dev_priv->gt.awake); /* Must be idle switching park/unpark */
for_each_engine(engine, dev_priv, id) {
struct intel_engine_execlists * const execlists =
&engine->execlists;
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-04-09 11:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-09 10:18 [PATCH] drm/i915: Park before resetting the submission backend Chris Wilson
2018-04-09 10:38 ` Sagar Arun Kamble
2018-04-09 10:41 ` Chris Wilson
2018-04-09 10:49 ` Michal Wajdeczko
2018-04-09 11:01 ` ✗ Fi.CI.BAT: warning for drm/i915: Park before resetting the submission backend (rev2) Patchwork
-- strict thread matches above, loose matches on Subject: below --
2018-04-05 10:57 [PATCH] drm/i915: Park before resetting the submission backend Chris Wilson
2018-04-05 11:03 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.