* [PATCH 1/3] drm/i915: Move calling engine->init_hw() to its own function
@ 2017-02-07 21:12 Chris Wilson
2017-02-07 21:12 ` [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Chris Wilson @ 2017-02-07 21:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
Just a simple refactor to move a loop and some support code out of
i915_gem_init_hw(). This is in preparation for avoiding a race between
the tasklet writing to ELSP whilst simultaneously being written by
engine->init_hw() following a GPU reset.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8a510c7f6828..17bcec9f1321 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4369,11 +4369,24 @@ static void init_unused_rings(struct drm_i915_private *dev_priv)
}
}
-int
-i915_gem_init_hw(struct drm_i915_private *dev_priv)
+static int __i915_gem_restart_engines(void *data)
{
+ struct drm_i915_private *i915 = data;
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ int err;
+
+ for_each_engine(engine, i915, id) {
+ err = engine->init_hw(engine);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+int i915_gem_init_hw(struct drm_i915_private *dev_priv)
+{
int ret;
dev_priv->gt.last_init_time = ktime_get();
@@ -4419,11 +4432,9 @@ i915_gem_init_hw(struct drm_i915_private *dev_priv)
}
/* Need to do basic initialisation of all rings first: */
- for_each_engine(engine, dev_priv, id) {
- ret = engine->init_hw(engine);
- if (ret)
- goto out;
- }
+ ret = __i915_gem_restart_engines(dev_priv);
+ if (ret)
+ goto out;
intel_mocs_init_l3cc_table(dev_priv);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases 2017-02-07 21:12 [PATCH 1/3] drm/i915: Move calling engine->init_hw() to its own function Chris Wilson @ 2017-02-07 21:12 ` Chris Wilson 2017-02-07 21:23 ` Chris Wilson 2017-02-07 21:12 ` [PATCH 3/3] drm/i915: Disable engine->irq_tasklet around resets Chris Wilson 2017-02-07 22:52 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Move calling engine->init_hw() to its own function Patchwork 2 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2017-02-07 21:12 UTC (permalink / raw) To: intel-gfx; +Cc: Mika Kuoppala Currently we do a reset prepare/finish around the call to reset the GPU, but it looks like we need a later stage after the hw has been reinitialised to allow GEM to restart itself. Start by splitting the 2 GEM phases into 3: prepare - before the reset, check if GEM recovered, then stop GEM reset - after the reset, update GEM bookkeeping finish - after the re-initialisation following the reset, restart GEM Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 ++- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 11 ++++++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index acbd772837b5..0aa4ac2b43ca 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1836,7 +1836,7 @@ void i915_reset(struct drm_i915_private *dev_priv) goto error; } - i915_gem_reset_finish(dev_priv); + i915_gem_reset(dev_priv); intel_overlay_reset(dev_priv); /* Ok, now get things going again... */ @@ -1859,6 +1859,7 @@ void i915_reset(struct drm_i915_private *dev_priv) goto error; } + i915_gem_reset_finish(dev_priv); i915_queue_hangcheck(dev_priv); wakeup: diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0cbd289da4ba..972d35259883 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3353,6 +3353,7 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error) } int i915_gem_reset_prepare(struct drm_i915_private *dev_priv); +void i915_gem_reset(struct drm_i915_private *dev_priv); void i915_gem_reset_finish(struct drm_i915_private *dev_priv); void i915_gem_set_wedged(struct drm_i915_private *dev_priv); void i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 17bcec9f1321..e125c0d61ce9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2760,7 +2760,7 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine) engine_skip_context(request); } -void i915_gem_reset_finish(struct drm_i915_private *dev_priv) +void i915_gem_reset(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; enum intel_engine_id id; @@ -2772,8 +2772,6 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv) for_each_engine(engine, dev_priv, id) i915_gem_reset_engine(engine); - i915_gem_restore_fences(dev_priv); - if (dev_priv->gt.awake) { intel_sanitize_gt_powersave(dev_priv); intel_enable_gt_powersave(dev_priv); @@ -2782,6 +2780,13 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv) } } +void i915_gem_reset_finish(struct drm_i915_private *dev_priv) +{ + lockdep_assert_held(&dev_priv->drm.struct_mutex); + + i915_gem_restore_fences(dev_priv); +} + static void nop_submit_request(struct drm_i915_gem_request *request) { dma_fence_set_error(&request->fence, -EIO); -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases 2017-02-07 21:12 ` [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases Chris Wilson @ 2017-02-07 21:23 ` Chris Wilson 0 siblings, 0 replies; 5+ messages in thread From: Chris Wilson @ 2017-02-07 21:23 UTC (permalink / raw) To: intel-gfx; +Cc: Mika Kuoppala On Tue, Feb 07, 2017 at 09:12:25PM +0000, Chris Wilson wrote: > Currently we do a reset prepare/finish around the call to reset the GPU, > but it looks like we need a later stage after the hw has been > reinitialised to allow GEM to restart itself. Start by splitting the 2 > GEM phases into 3: > > prepare - before the reset, check if GEM recovered, then stop GEM > > reset - after the reset, update GEM bookkeeping > > finish - after the re-initialisation following the reset, restart GEM > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 3 ++- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem.c | 11 ++++++++--- > 3 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index acbd772837b5..0aa4ac2b43ca 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1836,7 +1836,7 @@ void i915_reset(struct drm_i915_private *dev_priv) > goto error; > } > > - i915_gem_reset_finish(dev_priv); > + i915_gem_reset(dev_priv); > intel_overlay_reset(dev_priv); > > /* Ok, now get things going again... */ > @@ -1859,6 +1859,7 @@ void i915_reset(struct drm_i915_private *dev_priv) > goto error; > } > > + i915_gem_reset_finish(dev_priv); > i915_queue_hangcheck(dev_priv); > > wakeup: > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 0cbd289da4ba..972d35259883 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3353,6 +3353,7 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error) > } > > int i915_gem_reset_prepare(struct drm_i915_private *dev_priv); > +void i915_gem_reset(struct drm_i915_private *dev_priv); > void i915_gem_reset_finish(struct drm_i915_private *dev_priv); > void i915_gem_set_wedged(struct drm_i915_private *dev_priv); > void i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 17bcec9f1321..e125c0d61ce9 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2760,7 +2760,7 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine) > engine_skip_context(request); > } > > -void i915_gem_reset_finish(struct drm_i915_private *dev_priv) > +void i915_gem_reset(struct drm_i915_private *dev_priv) > { > struct intel_engine_cs *engine; > enum intel_engine_id id; > @@ -2772,8 +2772,6 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv) > for_each_engine(engine, dev_priv, id) > i915_gem_reset_engine(engine); > > - i915_gem_restore_fences(dev_priv); Restore fences has to be before the init_hw() for gen2/3 - otherwise we may restart requests trying to access through the fences. Imagine this patch just introduced the empty function and renamed the pair. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] drm/i915: Disable engine->irq_tasklet around resets 2017-02-07 21:12 [PATCH 1/3] drm/i915: Move calling engine->init_hw() to its own function Chris Wilson 2017-02-07 21:12 ` [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases Chris Wilson @ 2017-02-07 21:12 ` Chris Wilson 2017-02-07 22:52 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Move calling engine->init_hw() to its own function Patchwork 2 siblings, 0 replies; 5+ messages in thread From: Chris Wilson @ 2017-02-07 21:12 UTC (permalink / raw) To: intel-gfx; +Cc: Mika Kuoppala When we restart the engines, and we have active requests, a request on the first engine may complete and queue a request to the second engine before we try to restart the second engine. That queueing of the request may race with the engine to restart, and so may corrupt the current state. Disabling the engine->irq_tasklet prevents the two paths from writing into ELSP simultaneously (and modifyin the execlists_port[] at the same time). Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests") Testcase: igt/gem_exec_fence/await-hang Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index e125c0d61ce9..62fbc1d97ec5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2641,6 +2641,15 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv) for_each_engine(engine, dev_priv, id) { struct drm_i915_gem_request *request; + /* Prevent request submission to the hardware until we have + * completed the reset in i915_gem_reset_finish(). If a request + * is completed by one engine, it may then queue a request + * to a second via its engine->irq_tasklet *just* as we are + * calling engine->init_hw() and also writing the ELSP. + * Turning off the engine->irq_tasklet until the reset is over + * prevents the race. + */ + tasklet_disable(&engine->irq_tasklet); tasklet_kill(&engine->irq_tasklet); if (engine_stalled(engine)) { @@ -2782,9 +2791,15 @@ void i915_gem_reset(struct drm_i915_private *dev_priv) void i915_gem_reset_finish(struct drm_i915_private *dev_priv) { + struct intel_engine_cs *engine; + enum intel_engine_id id; + lockdep_assert_held(&dev_priv->drm.struct_mutex); i915_gem_restore_fences(dev_priv); + + for_each_engine(engine, dev_priv, id) + tasklet_enable(&engine->irq_tasklet); } static void nop_submit_request(struct drm_i915_gem_request *request) -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Move calling engine->init_hw() to its own function 2017-02-07 21:12 [PATCH 1/3] drm/i915: Move calling engine->init_hw() to its own function Chris Wilson 2017-02-07 21:12 ` [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases Chris Wilson 2017-02-07 21:12 ` [PATCH 3/3] drm/i915: Disable engine->irq_tasklet around resets Chris Wilson @ 2017-02-07 22:52 ` Patchwork 2 siblings, 0 replies; 5+ messages in thread From: Patchwork @ 2017-02-07 22:52 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: series starting with [1/3] drm/i915: Move calling engine->init_hw() to its own function URL : https://patchwork.freedesktop.org/series/19263/ State : success == Summary == Series 19263v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/19263/revisions/1/mbox/ fi-bdw-5557u total:252 pass:238 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:252 pass:213 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:252 pass:230 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:83 pass:70 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:252 pass:225 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:252 pass:221 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:252 pass:233 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:252 pass:233 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:252 pass:199 dwarn:0 dfail:0 fail:0 skip:53 fi-ivb-3520m total:252 pass:231 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:252 pass:231 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:252 pass:229 dwarn:0 dfail:0 fail:2 skip:21 fi-skl-6260u total:252 pass:239 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hq total:252 pass:232 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:252 pass:227 dwarn:4 dfail:0 fail:0 skip:21 fi-skl-6770hq total:252 pass:239 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:252 pass:221 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:252 pass:220 dwarn:0 dfail:0 fail:0 skip:32 7f1b128ee8d8cdd07a558d77e914dd99cc641b0f drm-tip: 2017y-02m-07d-21h-44m-07s UTC integration manifest 92cda5a drm/i915: Move calling engine->init_hw() to its own function == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3730/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-02-07 22:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-07 21:12 [PATCH 1/3] drm/i915: Move calling engine->init_hw() to its own function Chris Wilson 2017-02-07 21:12 ` [PATCH 2/3] drm/i915: Split GEM resetting into 3 phases Chris Wilson 2017-02-07 21:23 ` Chris Wilson 2017-02-07 21:12 ` [PATCH 3/3] drm/i915: Disable engine->irq_tasklet around resets Chris Wilson 2017-02-07 22:52 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Move calling engine->init_hw() to its own function Patchwork
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.