From 6e16d028e441b0b2c141aaecb39f4838cd2964b5 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Wed, 16 Nov 2016 17:20:29 +0200 Subject: drm/i915: Split up hangcheck phases In order to simplify hangcheck state keeping, split hangcheck per engine loop in three phases: state load, action, state save. Add few more hangcheck actions to separate between seqno, head and subunit movements. This helps to gather all the hangcheck actions under a single switch umbrella. Cc: Chris Wilson Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.h') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 3466b4e77e7c..3152b2b4a202 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -67,7 +67,9 @@ struct intel_hw_status_page { enum intel_engine_hangcheck_action { HANGCHECK_IDLE = 0, HANGCHECK_WAIT, - HANGCHECK_ACTIVE, + HANGCHECK_ACTIVE_SEQNO, + HANGCHECK_ACTIVE_HEAD, + HANGCHECK_ACTIVE_SUBUNITS, HANGCHECK_KICK, HANGCHECK_HUNG, }; -- cgit From 3fe3b030bd2d7a51c12aa6fe0e5178b9f1a726ec Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Fri, 18 Nov 2016 15:09:04 +0200 Subject: drm/i915: Decouple hang detection from hangcheck period Hangcheck state accumulation has gained more steps along the years, like head movement and more recently the subunit inactivity check. As the subunit sampling is only done if the previous state check showed inactivity, we have added more stages (and time) to reach a hang verdict. Asymmetric engine states led to different actual weight of 'one hangcheck unit' and it was demonstrated in some hangs that due to difference in stages, simpler engines were accused falsely of a hang as their scoring was much more quicker to accumulate above the hang treshold. To completely decouple the hangcheck guilty score from the hangcheck period, convert hangcheck score to a rough period of inactivity measurement. As these are tracked as jiffies, they are meaningful also across reset boundaries. This makes finding a guilty engine more accurate across multi engine activity scenarios, especially across asymmetric engines. We lose the ability to detect cross batch malicious attempts to hinder the progress. Plan is to move this functionality to be part of context banning which is more natural fit, later in the series. v2: use time_before macros (Chris) reinstate the pardoning of moving engine after hc (Chris) v3: avoid global state for per engine stall detection (Chris) v4: take timeline last retirement into account (Chris) v5: do debug print on pardoning, split out retirement timestamp (Chris) Cc: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_debugfs.c | 17 +++-- drivers/gpu/drm/i915/i915_drv.h | 6 +- drivers/gpu/drm/i915/i915_gem.c | 8 ++- drivers/gpu/drm/i915/i915_gpu_error.c | 46 ++++---------- drivers/gpu/drm/i915/intel_hangcheck.c | 108 +++++++++++++++----------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 40 +++++++++--- 6 files changed, 117 insertions(+), 108 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.h') diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fb47efdfb448..437212a95b19 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1351,10 +1351,12 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) seq_printf(m, "\tseqno = %x [current %x, last %x]\n", engine->hangcheck.seqno, seqno[id], intel_engine_last_submit(engine)); - seq_printf(m, "\twaiters? %s, fake irq active? %s\n", + seq_printf(m, "\twaiters? %s, fake irq active? %s, stalled? %s\n", yesno(intel_engine_has_waiter(engine)), yesno(test_bit(engine->id, - &dev_priv->gpu_error.missed_irq_rings))); + &dev_priv->gpu_error.missed_irq_rings)), + yesno(engine->hangcheck.stalled)); + spin_lock_irq(&b->lock); for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) { struct intel_wait *w = container_of(rb, typeof(*w), node); @@ -1367,8 +1369,11 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n", (long long)engine->hangcheck.acthd, (long long)acthd[id]); - seq_printf(m, "\tscore = %d\n", engine->hangcheck.score); - seq_printf(m, "\taction = %d\n", engine->hangcheck.action); + seq_printf(m, "\taction = %s(%d) %d ms ago\n", + hangcheck_action_to_str(engine->hangcheck.action), + engine->hangcheck.action, + jiffies_to_msecs(jiffies - + engine->hangcheck.action_timestamp)); if (engine->id == RCS) { seq_puts(m, "\tinstdone read =\n"); @@ -3162,11 +3167,11 @@ static int i915_engine_info(struct seq_file *m, void *unused) u64 addr; seq_printf(m, "%s\n", engine->name); - seq_printf(m, "\tcurrent seqno %x, last %x, hangcheck %x [score %d]\n", + seq_printf(m, "\tcurrent seqno %x, last %x, hangcheck %x [%d ms]\n", intel_engine_get_seqno(engine), intel_engine_last_submit(engine), engine->hangcheck.seqno, - engine->hangcheck.score); + jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp)); rcu_read_lock(); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 56002a52936d..0ebec2b77ae1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -800,7 +800,8 @@ struct drm_i915_error_state { /* Software tracked state */ bool waiting; int num_waiters; - int hangcheck_score; + unsigned long hangcheck_timestamp; + bool hangcheck_stalled; enum intel_engine_hangcheck_action hangcheck_action; struct i915_address_space *vm; int num_requests; @@ -1446,6 +1447,9 @@ struct i915_error_state_file_priv { #define I915_RESET_TIMEOUT (10 * HZ) /* 10s */ #define I915_FENCE_TIMEOUT (10 * HZ) /* 10s */ +#define I915_ENGINE_DEAD_TIMEOUT (4 * HZ) /* Seqno, head and subunits dead */ +#define I915_SEQNO_DEAD_TIMEOUT (12 * HZ) /* Seqno dead with active head */ + struct i915_gpu_error { /* For hangcheck timer */ #define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */ diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 902fa427c196..1f8dfd4aba61 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2705,9 +2705,13 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine) if (!request) return; - ring_hung = engine->hangcheck.score >= HANGCHECK_SCORE_RING_HUNG; - if (engine->hangcheck.seqno != intel_engine_get_seqno(engine)) + ring_hung = engine->hangcheck.stalled; + if (engine->hangcheck.seqno != intel_engine_get_seqno(engine)) { + DRM_DEBUG_DRIVER("%s pardoned, was guilty? %s\n", + engine->name, + yesno(ring_hung)); ring_hung = false; + } i915_set_reset_status(request->ctx, ring_hung); if (!ring_hung) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 4bcf1a0f5675..d5a4ec9b507f 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -316,28 +316,6 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, } } -static const char *hangcheck_action_to_str(enum intel_engine_hangcheck_action a) -{ - switch (a) { - case HANGCHECK_IDLE: - return "idle"; - case HANGCHECK_WAIT: - return "wait"; - case HANGCHECK_ACTIVE_SEQNO: - return "active seqno"; - case HANGCHECK_ACTIVE_HEAD: - return "active head"; - case HANGCHECK_ACTIVE_SUBUNITS: - return "active subunits"; - case HANGCHECK_KICK: - return "kick"; - case HANGCHECK_HUNG: - return "hung"; - } - - return "unknown"; -} - static void error_print_instdone(struct drm_i915_error_state_buf *m, struct drm_i915_error_engine *ee) { @@ -445,9 +423,13 @@ static void error_print_engine(struct drm_i915_error_state_buf *m, err_printf(m, " waiting: %s\n", yesno(ee->waiting)); err_printf(m, " ring->head: 0x%08x\n", ee->cpu_ring_head); err_printf(m, " ring->tail: 0x%08x\n", ee->cpu_ring_tail); - err_printf(m, " hangcheck: %s [%d]\n", - hangcheck_action_to_str(ee->hangcheck_action), - ee->hangcheck_score); + err_printf(m, " hangcheck stall: %s\n", yesno(ee->hangcheck_stalled)); + err_printf(m, " hangcheck action: %s\n", + hangcheck_action_to_str(ee->hangcheck_action)); + err_printf(m, " hangcheck action timestamp: %lu, %u ms ago\n", + ee->hangcheck_timestamp, + jiffies_to_msecs(jiffies - ee->hangcheck_timestamp)); + error_print_request(m, " ELSP[0]: ", &ee->execlist[0]); error_print_request(m, " ELSP[1]: ", &ee->execlist[1]); } @@ -536,7 +518,6 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, struct pci_dev *pdev = dev_priv->drm.pdev; struct drm_i915_error_state *error = error_priv->error; struct drm_i915_error_object *obj; - int max_hangcheck_score; int i, j; if (!error) { @@ -553,13 +534,9 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, err_printf(m, "Uptime: %ld s %ld us\n", error->uptime.tv_sec, error->uptime.tv_usec); err_print_capabilities(m, &error->device_info); - max_hangcheck_score = 0; - for (i = 0; i < ARRAY_SIZE(error->engine); i++) { - if (error->engine[i].hangcheck_score > max_hangcheck_score) - max_hangcheck_score = error->engine[i].hangcheck_score; - } + for (i = 0; i < ARRAY_SIZE(error->engine); i++) { - if (error->engine[i].hangcheck_score == max_hangcheck_score && + if (error->engine[i].hangcheck_stalled && error->engine[i].pid != -1) { err_printf(m, "Active process (on ring %s): %s [%d]\n", engine_str(i), @@ -945,7 +922,7 @@ static uint32_t i915_error_generate_code(struct drm_i915_private *dev_priv, * strictly a client bug. Use instdone to differentiate those some. */ for (i = 0; i < I915_NUM_ENGINES; i++) { - if (error->engine[i].hangcheck_action == HANGCHECK_HUNG) { + if (error->engine[i].hangcheck_stalled) { if (engine_id) *engine_id = i; @@ -1163,8 +1140,9 @@ static void error_record_engine_registers(struct drm_i915_error_state *error, ee->hws = I915_READ(mmio); } - ee->hangcheck_score = engine->hangcheck.score; + ee->hangcheck_timestamp = engine->hangcheck.action_timestamp; ee->hangcheck_action = engine->hangcheck.action; + ee->hangcheck_stalled = engine->hangcheck.stalled; if (USES_PPGTT(dev_priv)) { int i; diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index 3d2e81c81252..c03db022a6d8 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -236,13 +236,13 @@ head_stuck(struct intel_engine_cs *engine, u64 acthd) memset(&engine->hangcheck.instdone, 0, sizeof(engine->hangcheck.instdone)); - return HANGCHECK_ACTIVE_HEAD; + return ENGINE_ACTIVE_HEAD; } if (!subunits_stuck(engine)) - return HANGCHECK_ACTIVE_SUBUNITS; + return ENGINE_ACTIVE_SUBUNITS; - return HANGCHECK_HUNG; + return ENGINE_DEAD; } static enum intel_engine_hangcheck_action @@ -253,11 +253,11 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd) u32 tmp; ha = head_stuck(engine, acthd); - if (ha != HANGCHECK_HUNG) + if (ha != ENGINE_DEAD) return ha; if (IS_GEN2(dev_priv)) - return HANGCHECK_HUNG; + return ENGINE_DEAD; /* Is the chip hanging on a WAIT_FOR_EVENT? * If so we can simply poke the RB_WAIT bit @@ -270,25 +270,25 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd) "Kicking stuck wait on %s", engine->name); I915_WRITE_CTL(engine, tmp); - return HANGCHECK_KICK; + return ENGINE_WAIT_KICK; } if (INTEL_GEN(dev_priv) >= 6 && tmp & RING_WAIT_SEMAPHORE) { switch (semaphore_passed(engine)) { default: - return HANGCHECK_HUNG; + return ENGINE_DEAD; case 1: i915_handle_error(dev_priv, 0, "Kicking stuck semaphore on %s", engine->name); I915_WRITE_CTL(engine, tmp); - return HANGCHECK_KICK; + return ENGINE_WAIT_KICK; case 0: - return HANGCHECK_WAIT; + return ENGINE_WAIT; } } - return HANGCHECK_HUNG; + return ENGINE_DEAD; } static void hangcheck_load_sample(struct intel_engine_cs *engine, @@ -306,7 +306,6 @@ static void hangcheck_load_sample(struct intel_engine_cs *engine, hc->acthd = intel_engine_get_active_head(engine); hc->seqno = intel_engine_get_seqno(engine); - hc->score = engine->hangcheck.score; } static void hangcheck_store_sample(struct intel_engine_cs *engine, @@ -314,8 +313,8 @@ static void hangcheck_store_sample(struct intel_engine_cs *engine, { engine->hangcheck.acthd = hc->acthd; engine->hangcheck.seqno = hc->seqno; - engine->hangcheck.score = hc->score; engine->hangcheck.action = hc->action; + engine->hangcheck.stalled = hc->stalled; } static enum intel_engine_hangcheck_action @@ -323,10 +322,10 @@ hangcheck_get_action(struct intel_engine_cs *engine, const struct intel_engine_hangcheck *hc) { if (engine->hangcheck.seqno != hc->seqno) - return HANGCHECK_ACTIVE_SEQNO; + return ENGINE_ACTIVE_SEQNO; if (i915_seqno_passed(hc->seqno, intel_engine_last_submit(engine))) - return HANGCHECK_IDLE; + return ENGINE_IDLE; return engine_stuck(engine, hc->acthd); } @@ -334,60 +333,57 @@ hangcheck_get_action(struct intel_engine_cs *engine, static void hangcheck_accumulate_sample(struct intel_engine_cs *engine, struct intel_engine_hangcheck *hc) { + unsigned long timeout = I915_ENGINE_DEAD_TIMEOUT; + hc->action = hangcheck_get_action(engine, hc); - switch (hc->action) { - case HANGCHECK_IDLE: - case HANGCHECK_WAIT: - break; + /* We always increment the progress + * if the engine is busy and still processing + * the same request, so that no single request + * can run indefinitely (such as a chain of + * batches). The only time we do not increment + * the hangcheck score on this ring, if this + * engine is in a legitimate wait for another + * engine. In that case the waiting engine is a + * victim and we want to be sure we catch the + * right culprit. Then every time we do kick + * the ring, make it as a progress as the seqno + * advancement might ensure and if not, it + * will catch the hanging engine. + */ - case HANGCHECK_ACTIVE_HEAD: - case HANGCHECK_ACTIVE_SUBUNITS: - /* We always increment the hangcheck score - * if the engine is busy and still processing - * the same request, so that no single request - * can run indefinitely (such as a chain of - * batches). The only time we do not increment - * the hangcheck score on this ring, if this - * engine is in a legitimate wait for another - * engine. In that case the waiting engine is a - * victim and we want to be sure we catch the - * right culprit. Then every time we do kick - * the ring, add a small increment to the - * score so that we can catch a batch that is - * being repeatedly kicked and so responsible - * for stalling the machine. - */ - hc->score += 1; - break; + switch (hc->action) { + case ENGINE_IDLE: + case ENGINE_ACTIVE_SEQNO: + /* Clear head and subunit states on seqno movement */ + hc->acthd = 0; - case HANGCHECK_KICK: - hc->score += 5; - break; + memset(&engine->hangcheck.instdone, 0, + sizeof(engine->hangcheck.instdone)); - case HANGCHECK_HUNG: - hc->score += 20; + /* Intentional fall through */ + case ENGINE_WAIT_KICK: + case ENGINE_WAIT: + engine->hangcheck.action_timestamp = jiffies; break; - case HANGCHECK_ACTIVE_SEQNO: - /* Gradually reduce the count so that we catch DoS - * attempts across multiple batches. + case ENGINE_ACTIVE_HEAD: + case ENGINE_ACTIVE_SUBUNITS: + /* Seqno stuck with still active engine gets leeway, + * in hopes that it is just a long shader. */ - if (hc->score > 0) - hc->score -= 15; - if (hc->score < 0) - hc->score = 0; - - /* Clear head and subunit states on seqno movement */ - hc->acthd = 0; + timeout = I915_SEQNO_DEAD_TIMEOUT; + break; - memset(&engine->hangcheck.instdone, 0, - sizeof(engine->hangcheck.instdone)); + case ENGINE_DEAD: break; default: MISSING_CASE(hc->action); } + + hc->stalled = time_after(jiffies, + engine->hangcheck.action_timestamp + timeout); } static void hangcheck_declare_hang(struct drm_i915_private *i915, @@ -454,9 +450,9 @@ static void i915_hangcheck_elapsed(struct work_struct *work) hangcheck_accumulate_sample(engine, hc); hangcheck_store_sample(engine, hc); - if (hc->score >= HANGCHECK_SCORE_RING_HUNG) { + if (engine->hangcheck.stalled) { hung |= intel_engine_flag(engine); - if (hc->action != HANGCHECK_HUNG) + if (hc->action != ENGINE_DEAD) stuck |= intel_engine_flag(engine); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 3152b2b4a202..3f43adefd1c0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -65,16 +65,37 @@ struct intel_hw_status_page { GEN8_SEMAPHORE_OFFSET(from, (__ring)->id)) enum intel_engine_hangcheck_action { - HANGCHECK_IDLE = 0, - HANGCHECK_WAIT, - HANGCHECK_ACTIVE_SEQNO, - HANGCHECK_ACTIVE_HEAD, - HANGCHECK_ACTIVE_SUBUNITS, - HANGCHECK_KICK, - HANGCHECK_HUNG, + ENGINE_IDLE = 0, + ENGINE_WAIT, + ENGINE_ACTIVE_SEQNO, + ENGINE_ACTIVE_HEAD, + ENGINE_ACTIVE_SUBUNITS, + ENGINE_WAIT_KICK, + ENGINE_DEAD, }; -#define HANGCHECK_SCORE_RING_HUNG 31 +static inline const char * +hangcheck_action_to_str(const enum intel_engine_hangcheck_action a) +{ + switch (a) { + case ENGINE_IDLE: + return "idle"; + case ENGINE_WAIT: + return "wait"; + case ENGINE_ACTIVE_SEQNO: + return "active seqno"; + case ENGINE_ACTIVE_HEAD: + return "active head"; + case ENGINE_ACTIVE_SUBUNITS: + return "active subunits"; + case ENGINE_WAIT_KICK: + return "wait kick"; + case ENGINE_DEAD: + return "dead"; + } + + return "unknown"; +} #define I915_MAX_SLICES 3 #define I915_MAX_SUBSLICES 3 @@ -106,10 +127,11 @@ struct intel_instdone { struct intel_engine_hangcheck { u64 acthd; u32 seqno; - int score; enum intel_engine_hangcheck_action action; + unsigned long action_timestamp; int deadlock; struct intel_instdone instdone; + bool stalled; }; struct intel_ring { -- cgit From e8a9c58fcd9a5081f71f57f370af1347ed6a310b Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 18 Dec 2016 15:37:20 +0000 Subject: drm/i915: Unify active context tracking between legacy/execlists/guc The requests conversion introduced a nasty bug where we could generate a new request in the middle of constructing a request if we needed to idle the system in order to evict space for a context. The request to idle would be executed (and waited upon) before the current one, creating a minor havoc in the seqno accounting, as we will consider the current request to already be completed (prior to deferred seqno assignment) but ring->last_retired_head would have been updated and still could allow us to overwrite the current request before execution. We also employed two different mechanisms to track the active context until it was switched out. The legacy method allowed for waiting upon an active context (it could forcibly evict any vma, including context's), but the execlists method took a step backwards by pinning the vma for the entire active lifespan of the context (the only way to evict was to idle the entire GPU, not individual contexts). However, to circumvent the tricky issue of locking (i.e. we cannot take struct_mutex at the time of i915_gem_request_submit(), where we would want to move the previous context onto the active tracker and unpin it), we take the execlists approach and keep the contexts pinned until retirement. The benefit of the execlists approach, more important for execlists than legacy, was the reduction in work in pinning the context for each request - as the context was kept pinned until idle, it could short circuit the pinning for all active contexts. We introduce new engine vfuncs to pin and unpin the context respectively. The context is pinned at the start of the request, and only unpinned when the following request is retired (this ensures that the context is idle and coherent in main memory before we unpin it). We move the engine->last_context tracking into the retirement itself (rather than during request submission) in order to allow the submission to be reordered or unwound without undue difficultly. And finally an ulterior motive for unifying context handling was to prepare for mock requests. v2: Rename to last_retired_context, split out legacy_context tracking for MI_SET_CONTEXT. Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20161218153724.8439-3-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 4 -- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_gem_context.c | 110 +++++------------------------ drivers/gpu/drm/i915/i915_gem_request.c | 38 ++++++---- drivers/gpu/drm/i915/i915_gem_request.h | 11 --- drivers/gpu/drm/i915/i915_guc_submission.c | 11 --- drivers/gpu/drm/i915/i915_perf.c | 18 ++--- drivers/gpu/drm/i915/intel_display.c | 3 +- drivers/gpu/drm/i915/intel_engine_cs.c | 21 +++++- drivers/gpu/drm/i915/intel_lrc.c | 62 ++++++---------- drivers/gpu/drm/i915/intel_lrc.h | 5 +- drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 58 +++++++++------ drivers/gpu/drm/i915/intel_ringbuffer.h | 23 +++++- 14 files changed, 151 insertions(+), 217 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.h') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 085c8ecd09b5..c89b012af914 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2446,7 +2446,6 @@ struct drm_i915_private { struct i915_perf_stream *exclusive_stream; u32 specific_ctx_id; - struct i915_vma *pinned_rcs_vma; struct hrtimer poll_check_timer; wait_queue_head_t poll_wq; @@ -3488,9 +3487,6 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file); void i915_gem_context_close(struct drm_device *dev, struct drm_file *file); int i915_switch_context(struct drm_i915_gem_request *req); int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv); -struct i915_vma * -i915_gem_context_pin_legacy(struct i915_gem_context *ctx, - unsigned int flags); void i915_gem_context_free(struct kref *ctx_ref); struct i915_gem_context * i915_gem_context_create_gvt(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 782be625d37d..9b308af89ada 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4206,7 +4206,7 @@ static void assert_kernel_context_is_current(struct drm_i915_private *dev_priv) enum intel_engine_id id; for_each_engine(engine, dev_priv, id) - GEM_BUG_ON(engine->last_context != dev_priv->kernel_context); + GEM_BUG_ON(engine->last_retired_context != dev_priv->kernel_context); } int i915_gem_suspend(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a57c22659a3c..598a70d2b695 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -416,21 +416,6 @@ out: return ctx; } -static void i915_gem_context_unpin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) -{ - if (i915.enable_execlists) { - intel_lr_context_unpin(ctx, engine); - } else { - struct intel_context *ce = &ctx->engine[engine->id]; - - if (ce->state) - i915_vma_unpin(ce->state); - - i915_gem_context_put(ctx); - } -} - int i915_gem_context_init(struct drm_i915_private *dev_priv) { struct i915_gem_context *ctx; @@ -490,10 +475,13 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv) lockdep_assert_held(&dev_priv->drm.struct_mutex); for_each_engine(engine, dev_priv, id) { - if (engine->last_context) { - i915_gem_context_unpin(engine->last_context, engine); - engine->last_context = NULL; - } + engine->legacy_active_context = NULL; + + if (!engine->last_retired_context) + continue; + + engine->context_unpin(engine, engine->last_retired_context); + engine->last_retired_context = NULL; } /* Force the GPU state to be restored on enabling */ @@ -715,7 +703,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt, if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings)) return false; - return to == engine->last_context; + return to == engine->legacy_active_context; } static bool @@ -727,11 +715,11 @@ needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt, return false; /* Always load the ppgtt on first use */ - if (!engine->last_context) + if (!engine->legacy_active_context) return true; /* Same context without new entries, skip */ - if (engine->last_context == to && + if (engine->legacy_active_context == to && !(intel_engine_flag(engine) & ppgtt->pd_dirty_rings)) return false; @@ -761,57 +749,20 @@ needs_pd_load_post(struct i915_hw_ppgtt *ppgtt, return false; } -struct i915_vma * -i915_gem_context_pin_legacy(struct i915_gem_context *ctx, - unsigned int flags) -{ - struct i915_vma *vma = ctx->engine[RCS].state; - int ret; - - /* Clear this page out of any CPU caches for coherent swap-in/out. - * We only want to do this on the first bind so that we do not stall - * on an active context (which by nature is already on the GPU). - */ - if (!(vma->flags & I915_VMA_GLOBAL_BIND)) { - ret = i915_gem_object_set_to_gtt_domain(vma->obj, false); - if (ret) - return ERR_PTR(ret); - } - - ret = i915_vma_pin(vma, 0, ctx->ggtt_alignment, PIN_GLOBAL | flags); - if (ret) - return ERR_PTR(ret); - - return vma; -} - static int do_rcs_switch(struct drm_i915_gem_request *req) { struct i915_gem_context *to = req->ctx; struct intel_engine_cs *engine = req->engine; struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt; - struct i915_vma *vma; - struct i915_gem_context *from; + struct i915_gem_context *from = engine->legacy_active_context; u32 hw_flags; int ret, i; + GEM_BUG_ON(engine->id != RCS); + if (skip_rcs_switch(ppgtt, engine, to)) return 0; - /* Trying to pin first makes error handling easier. */ - vma = i915_gem_context_pin_legacy(to, 0); - if (IS_ERR(vma)) - return PTR_ERR(vma); - - /* - * Pin can switch back to the default context if we end up calling into - * evict_everything - as a last ditch gtt defrag effort that also - * switches to the default context. Hence we need to reload from here. - * - * XXX: Doing so is painfully broken! - */ - from = engine->last_context; - if (needs_pd_load_pre(ppgtt, engine, to)) { /* Older GENs and non render rings still want the load first, * "PP_DCLV followed by PP_DIR_BASE register through Load @@ -820,7 +771,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) trace_switch_mm(engine, to); ret = ppgtt->switch_mm(ppgtt, req); if (ret) - goto err; + return ret; } if (!to->engine[RCS].initialised || i915_gem_context_is_default(to)) @@ -837,29 +788,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) if (to != from || (hw_flags & MI_FORCE_RESTORE)) { ret = mi_set_context(req, hw_flags); if (ret) - goto err; - } + return ret; - /* The backing object for the context is done after switching to the - * *next* context. Therefore we cannot retire the previous context until - * the next context has already started running. In fact, the below code - * is a bit suboptimal because the retiring can occur simply after the - * MI_SET_CONTEXT instead of when the next seqno has completed. - */ - if (from != NULL) { - /* As long as MI_SET_CONTEXT is serializing, ie. it flushes the - * whole damn pipeline, we don't need to explicitly mark the - * object dirty. The only exception is that the context must be - * correct in case the object gets swapped out. Ideally we'd be - * able to defer doing this until we know the object would be - * swapped, but there is no way to do that yet. - */ - i915_vma_move_to_active(from->engine[RCS].state, req, 0); - /* state is kept alive until the next request */ - i915_vma_unpin(from->engine[RCS].state); - i915_gem_context_put(from); + engine->legacy_active_context = to; } - engine->last_context = i915_gem_context_get(to); /* GEN8 does *not* require an explicit reload if the PDPs have been * setup, and we do not wish to move them. @@ -900,10 +832,6 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) } return 0; - -err: - i915_vma_unpin(vma); - return ret; } /** @@ -943,12 +871,6 @@ int i915_switch_context(struct drm_i915_gem_request *req) ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); } - if (to != engine->last_context) { - if (engine->last_context) - i915_gem_context_put(engine->last_context); - engine->last_context = i915_gem_context_get(to); - } - return 0; } diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index fcf22b0e2967..475d557f2301 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -206,6 +206,7 @@ void i915_gem_retire_noop(struct i915_gem_active *active, static void i915_gem_request_retire(struct drm_i915_gem_request *request) { + struct intel_engine_cs *engine = request->engine; struct i915_gem_active *active, *next; lockdep_assert_held(&request->i915->drm.struct_mutex); @@ -216,9 +217,9 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) trace_i915_gem_request_retire(request); - spin_lock_irq(&request->engine->timeline->lock); + spin_lock_irq(&engine->timeline->lock); list_del_init(&request->link); - spin_unlock_irq(&request->engine->timeline->lock); + spin_unlock_irq(&engine->timeline->lock); /* We know the GPU must have read the request to have * sent us the seqno + interrupt, so use the position @@ -266,17 +267,20 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) i915_gem_request_remove_from_client(request); - if (request->previous_context) { - if (i915.enable_execlists) - intel_lr_context_unpin(request->previous_context, - request->engine); - } - /* Retirement decays the ban score as it is a sign of ctx progress */ if (request->ctx->ban_score > 0) request->ctx->ban_score--; - i915_gem_context_put(request->ctx); + /* The backing object for the context is done after switching to the + * *next* context. Therefore we cannot retire the previous context until + * the next context has already started running. However, since we + * cannot take the required locks at i915_gem_request_submit() we + * defer the unpinning of the active context to now, retirement of + * the subsequent request. + */ + if (engine->last_retired_context) + engine->context_unpin(engine, engine->last_retired_context); + engine->last_retired_context = request->ctx; dma_fence_signal(&request->fence); @@ -524,10 +528,18 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, if (ret) return ERR_PTR(ret); - ret = reserve_global_seqno(dev_priv); + /* Pinning the contexts may generate requests in order to acquire + * GGTT space, so do this first before we reserve a seqno for + * ourselves. + */ + ret = engine->context_pin(engine, ctx); if (ret) return ERR_PTR(ret); + ret = reserve_global_seqno(dev_priv); + if (ret) + goto err_unpin; + /* Move the oldest request to the slab-cache (if not in use!) */ req = list_first_entry_or_null(&engine->timeline->requests, typeof(*req), link); @@ -593,11 +605,10 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, INIT_LIST_HEAD(&req->active_list); req->i915 = dev_priv; req->engine = engine; - req->ctx = i915_gem_context_get(ctx); + req->ctx = ctx; /* No zalloc, must clear what we need by hand */ req->global_seqno = 0; - req->previous_context = NULL; req->file_priv = NULL; req->batch = NULL; @@ -633,10 +644,11 @@ err_ctx: GEM_BUG_ON(!list_empty(&req->priotree.signalers_list)); GEM_BUG_ON(!list_empty(&req->priotree.waiters_list)); - i915_gem_context_put(ctx); kmem_cache_free(dev_priv->requests, req); err_unreserve: dev_priv->gt.active_requests--; +err_unpin: + engine->context_unpin(engine, ctx); return ERR_PTR(ret); } diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h index e2b077df2da0..8569b35a332a 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.h +++ b/drivers/gpu/drm/i915/i915_gem_request.h @@ -170,17 +170,6 @@ struct drm_i915_gem_request { /** Preallocate space in the ring for the emitting the request */ u32 reserved_space; - /** - * Context related to the previous request. - * As the contexts are accessed by the hardware until the switch is - * completed to a new context, the hardware may still be writing - * to the context object after the breadcrumb is visible. We must - * not unpin/unbind/prune that object whilst still active and so - * we keep the previous context pinned until the following (this) - * request is retired. - */ - struct i915_gem_context *previous_context; - /** Batch buffer related to this request if any (used for * error state dump only). */ diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index a1232034f37a..3e20fe2be811 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -534,17 +534,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq) static void i915_guc_submit(struct drm_i915_gem_request *rq) { - struct intel_engine_cs *engine = rq->engine; - - /* We keep the previous context alive until we retire the following - * request. This ensures that any the context object is still pinned - * for any residual writes the HW makes into it on the context switch - * into the next object following the breadcrumb. Otherwise, we may - * retire the context too early. - */ - rq->previous_context = engine->last_context; - engine->last_context = rq->ctx; - i915_gem_request_submit(rq); __i915_guc_submit(rq); } diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index ae7bd0ed7b1a..da8537cb8136 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -743,7 +743,7 @@ static int i915_oa_read(struct i915_perf_stream *stream, static int oa_get_render_ctx_id(struct i915_perf_stream *stream) { struct drm_i915_private *dev_priv = stream->dev_priv; - struct i915_vma *vma; + struct intel_engine_cs *engine = dev_priv->engine[RCS]; int ret; ret = i915_mutex_lock_interruptible(&dev_priv->drm); @@ -755,19 +755,16 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) * * NB: implied RCS engine... */ - vma = i915_gem_context_pin_legacy(stream->ctx, 0); - if (IS_ERR(vma)) { - ret = PTR_ERR(vma); + ret = engine->context_pin(engine, stream->ctx); + if (ret) goto unlock; - } - - dev_priv->perf.oa.pinned_rcs_vma = vma; /* Explicitly track the ID (instead of calling i915_ggtt_offset() * on the fly) considering the difference with gen8+ and * execlists */ - dev_priv->perf.oa.specific_ctx_id = i915_ggtt_offset(vma); + dev_priv->perf.oa.specific_ctx_id = + i915_ggtt_offset(stream->ctx->engine[engine->id].state); unlock: mutex_unlock(&dev_priv->drm.struct_mutex); @@ -785,13 +782,12 @@ unlock: static void oa_put_render_ctx_id(struct i915_perf_stream *stream) { struct drm_i915_private *dev_priv = stream->dev_priv; + struct intel_engine_cs *engine = dev_priv->engine[RCS]; mutex_lock(&dev_priv->drm.struct_mutex); - i915_vma_unpin(dev_priv->perf.oa.pinned_rcs_vma); - dev_priv->perf.oa.pinned_rcs_vma = NULL; - dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; + engine->context_unpin(engine, stream->ctx); mutex_unlock(&dev_priv->drm.struct_mutex); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bc1af87789bc..9f356ad5329e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12295,7 +12295,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func); queue_work(system_unbound_wq, &work->mmio_work); } else { - request = i915_gem_request_alloc(engine, engine->last_context); + request = i915_gem_request_alloc(engine, + dev_priv->kernel_context); if (IS_ERR(request)) { ret = PTR_ERR(request); goto cleanup_unpin; diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index e8afe1185831..97bbbc3d6aa8 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -304,15 +304,30 @@ int intel_engine_init_common(struct intel_engine_cs *engine) { int ret; - ret = intel_engine_init_breadcrumbs(engine); + /* We may need to do things with the shrinker which + * require us to immediately switch back to the default + * context. This can cause a problem as pinning the + * default context also requires GTT space which may not + * be available. To avoid this we always pin the default + * context. + */ + ret = engine->context_pin(engine, engine->i915->kernel_context); if (ret) return ret; + ret = intel_engine_init_breadcrumbs(engine); + if (ret) + goto err_unpin; + ret = i915_gem_render_state_init(engine); if (ret) - return ret; + goto err_unpin; return 0; + +err_unpin: + engine->context_unpin(engine, engine->i915->kernel_context); + return ret; } /** @@ -330,6 +345,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) intel_engine_fini_breadcrumbs(engine); intel_engine_cleanup_cmd_parser(engine); i915_gem_batch_pool_fini(&engine->batch_pool); + + engine->context_unpin(engine, engine->i915->kernel_context); } u64 intel_engine_get_active_head(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index b848b5f205ce..599afedc4494 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -512,15 +512,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine) RB_CLEAR_NODE(&cursor->priotree.node); cursor->priotree.priority = INT_MAX; - /* We keep the previous context alive until we retire the - * following request. This ensures that any the context object - * is still pinned for any residual writes the HW makes into it - * on the context switch into the next object following the - * breadcrumb. Otherwise, we may retire the context too early. - */ - cursor->previous_context = engine->last_context; - engine->last_context = cursor->ctx; - __i915_gem_request_submit(cursor); last = cursor; submit = true; @@ -772,8 +763,8 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio) /* XXX Do we need to preempt to make room for us and our deps? */ } -static int intel_lr_context_pin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +static int execlists_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = &ctx->engine[engine->id]; void *vaddr; @@ -784,6 +775,12 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx, if (ce->pin_count++) return 0; + if (!ce->state) { + ret = execlists_context_deferred_alloc(ctx, engine); + if (ret) + goto err; + } + ret = i915_vma_pin(ce->state, 0, GEN8_LR_CONTEXT_ALIGN, PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL); if (ret) @@ -825,8 +822,8 @@ err: return ret; } -void intel_lr_context_unpin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +static void execlists_context_unpin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = &ctx->engine[engine->id]; @@ -850,24 +847,17 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request struct intel_context *ce = &request->ctx->engine[engine->id]; int ret; + GEM_BUG_ON(!ce->pin_count); + /* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just * have to repeat work. */ request->reserved_space += EXECLISTS_REQUEST_SIZE; - if (!ce->state) { - ret = execlists_context_deferred_alloc(request->ctx, engine); - if (ret) - return ret; - } - + GEM_BUG_ON(!ce->ring); request->ring = ce->ring; - ret = intel_lr_context_pin(request->ctx, engine); - if (ret) - return ret; - if (i915.enable_guc_submission) { /* * Check that the GuC has space for the request before @@ -876,7 +866,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request */ ret = i915_guc_wq_reserve(request); if (ret) - goto err_unpin; + goto err; } ret = intel_ring_begin(request, 0); @@ -904,8 +894,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request err_unreserve: if (i915.enable_guc_submission) i915_guc_wq_unreserve(request); -err_unpin: - intel_lr_context_unpin(request->ctx, engine); +err: return ret; } @@ -1789,13 +1778,12 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *engine) if (engine->cleanup) engine->cleanup(engine); - intel_engine_cleanup_common(engine); - if (engine->status_page.vma) { i915_gem_object_unpin_map(engine->status_page.vma->obj); engine->status_page.vma = NULL; } - intel_lr_context_unpin(dev_priv->kernel_context, engine); + + intel_engine_cleanup_common(engine); lrc_destroy_wa_ctx_obj(engine); engine->i915 = NULL; @@ -1820,6 +1808,10 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) /* Default vfuncs which can be overriden by each engine. */ engine->init_hw = gen8_init_common_ring; engine->reset_hw = reset_common_ring; + + engine->context_pin = execlists_context_pin; + engine->context_unpin = execlists_context_unpin; + engine->emit_flush = gen8_emit_flush; engine->emit_breadcrumb = gen8_emit_breadcrumb; engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_sz; @@ -1902,18 +1894,6 @@ logical_ring_init(struct intel_engine_cs *engine) if (ret) goto error; - ret = execlists_context_deferred_alloc(dctx, engine); - if (ret) - goto error; - - /* As this is the default context, always pin it */ - ret = intel_lr_context_pin(dctx, engine); - if (ret) { - DRM_ERROR("Failed to pin context for %s: %d\n", - engine->name, ret); - goto error; - } - /* And setup the hardware status page. */ ret = lrc_setup_hws(engine, dctx->engine[engine->id].state); if (ret) { diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 7c6403243394..b5630331086a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -79,13 +79,10 @@ int intel_engines_init(struct drm_i915_private *dev_priv); #define LRC_PPHWSP_PN (LRC_GUCSHR_PN + 1) #define LRC_STATE_PN (LRC_PPHWSP_PN + 1) +struct drm_i915_private; struct i915_gem_context; uint32_t intel_lr_context_size(struct intel_engine_cs *engine); -void intel_lr_context_unpin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine); - -struct drm_i915_private; void intel_lr_context_resume(struct drm_i915_private *dev_priv); uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 06e55967f180..1d4699d8fb10 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6792,7 +6792,7 @@ static void __intel_autoenable_gt_powersave(struct work_struct *work) goto out; rcs = dev_priv->engine[RCS]; - if (rcs->last_context) + if (rcs->last_retired_context) goto out; if (!rcs->init_context) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0b7d13b6e228..00ff572541b6 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1939,8 +1939,26 @@ intel_ring_free(struct intel_ring *ring) kfree(ring); } -static int intel_ring_context_pin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +static int context_pin(struct i915_gem_context *ctx, unsigned int flags) +{ + struct i915_vma *vma = ctx->engine[RCS].state; + int ret; + + /* Clear this page out of any CPU caches for coherent swap-in/out. + * We only want to do this on the first bind so that we do not stall + * on an active context (which by nature is already on the GPU). + */ + if (!(vma->flags & I915_VMA_GLOBAL_BIND)) { + ret = i915_gem_object_set_to_gtt_domain(vma->obj, false); + if (ret) + return ret; + } + + return i915_vma_pin(vma, 0, ctx->ggtt_alignment, PIN_GLOBAL | flags); +} + +static int intel_ring_context_pin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = &ctx->engine[engine->id]; int ret; @@ -1951,13 +1969,15 @@ static int intel_ring_context_pin(struct i915_gem_context *ctx, return 0; if (ce->state) { - struct i915_vma *vma; + unsigned int flags; + + flags = 0; + if (ctx == ctx->i915->kernel_context) + flags = PIN_HIGH; - vma = i915_gem_context_pin_legacy(ctx, PIN_HIGH); - if (IS_ERR(vma)) { - ret = PTR_ERR(vma); + ret = context_pin(ctx, flags); + if (ret) goto error; - } } /* The kernel context is only used as a placeholder for flushing the @@ -1978,12 +1998,13 @@ error: return ret; } -static void intel_ring_context_unpin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +static void intel_ring_context_unpin(struct intel_engine_cs *engine, + struct i915_gem_context *ctx) { struct intel_context *ce = &ctx->engine[engine->id]; lockdep_assert_held(&ctx->i915->drm.struct_mutex); + GEM_BUG_ON(ce->pin_count == 0); if (--ce->pin_count) return; @@ -2008,17 +2029,6 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine) if (ret) goto error; - /* We may need to do things with the shrinker which - * require us to immediately switch back to the default - * context. This can cause a problem as pinning the - * default context also requires GTT space which may not - * be available. To avoid this we always pin the default - * context. - */ - ret = intel_ring_context_pin(dev_priv->kernel_context, engine); - if (ret) - goto error; - ring = intel_engine_create_ring(engine, 32 * PAGE_SIZE); if (IS_ERR(ring)) { ret = PTR_ERR(ring); @@ -2077,8 +2087,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine) intel_engine_cleanup_common(engine); - intel_ring_context_unpin(dev_priv->kernel_context, engine); - engine->i915 = NULL; dev_priv->engine[engine->id] = NULL; kfree(engine); @@ -2099,12 +2107,15 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request) { int ret; + GEM_BUG_ON(!request->ctx->engine[request->engine->id].pin_count); + /* Flush enough space to reduce the likelihood of waiting after * we start building the request - in which case we will just * have to repeat work. */ request->reserved_space += LEGACY_REQUEST_SIZE; + GEM_BUG_ON(!request->engine->buffer); request->ring = request->engine->buffer; ret = intel_ring_begin(request, 0); @@ -2584,6 +2595,9 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv, engine->init_hw = init_ring_common; engine->reset_hw = reset_ring_common; + engine->context_pin = intel_ring_context_pin; + engine->context_unpin = intel_ring_context_unpin; + engine->emit_breadcrumb = i9xx_emit_breadcrumb; engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz; if (i915.semaphores) { diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 3f43adefd1c0..4f1271821fa9 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -266,6 +266,10 @@ struct intel_engine_cs { void (*reset_hw)(struct intel_engine_cs *engine, struct drm_i915_gem_request *req); + int (*context_pin)(struct intel_engine_cs *engine, + struct i915_gem_context *ctx); + void (*context_unpin)(struct intel_engine_cs *engine, + struct i915_gem_context *ctx); int (*init_context)(struct drm_i915_gem_request *req); int (*emit_flush)(struct drm_i915_gem_request *request, @@ -379,7 +383,24 @@ struct intel_engine_cs { bool preempt_wa; u32 ctx_desc_template; - struct i915_gem_context *last_context; + /* Contexts are pinned whilst they are active on the GPU. The last + * context executed remains active whilst the GPU is idle - the + * switch away and write to the context object only occurs on the + * next execution. Contexts are only unpinned on retirement of the + * following request ensuring that we can always write to the object + * on the context switch even after idling. Across suspend, we switch + * to the kernel context and trash it as the save may not happen + * before the hardware is powered down. + */ + struct i915_gem_context *last_retired_context; + + /* We track the current MI_SET_CONTEXT in order to eliminate + * redudant context switches. This presumes that requests are not + * reordered! Or when they are the tracking is updated along with + * the emission of individual requests into the legacy command + * stream (ring). + */ + struct i915_gem_context *legacy_active_context; struct intel_engine_hangcheck hangcheck; -- cgit From f73e73999d39a274adb8b342d7d8e722ffcf92d5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 18 Dec 2016 15:37:24 +0000 Subject: drm/i915: Swap if(enable_execlists) in i915_gem_request_alloc for a vfunc A fairly trivial move of a matching pair of routines (for preparing a request for construction) onto an engine vfunc. The ulterior motive is to be able to create a mock request implementation. Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20161218153724.8439-7-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_request.c | 5 +---- drivers/gpu/drm/i915/intel_lrc.c | 4 +++- drivers/gpu/drm/i915/intel_lrc.h | 2 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++- drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +-- 5 files changed, 8 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.h') diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 475d557f2301..7427aac74923 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -622,10 +622,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine, req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST; GEM_BUG_ON(req->reserved_space < engine->emit_breadcrumb_sz); - if (i915.enable_execlists) - ret = intel_logical_ring_alloc_request_extras(req); - else - ret = intel_ring_alloc_request_extras(req); + ret = engine->request_alloc(req); if (ret) goto err_ctx; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 1f8f35a94157..d322d3c11201 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -845,7 +845,7 @@ static void execlists_context_unpin(struct intel_engine_cs *engine, i915_gem_context_put(ctx); } -int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request) +static int execlists_request_alloc(struct drm_i915_gem_request *request) { struct intel_engine_cs *engine = request->engine; struct intel_context *ce = &request->ctx->engine[engine->id]; @@ -1816,6 +1816,8 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) engine->context_pin = execlists_context_pin; engine->context_unpin = execlists_context_unpin; + engine->request_alloc = execlists_request_alloc; + engine->emit_flush = gen8_emit_flush; engine->emit_breadcrumb = gen8_emit_breadcrumb; engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_sz; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index b5630331086a..01ba36ea125e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -63,8 +63,6 @@ enum { }; /* Logical Rings */ -int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request); -int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request); void intel_logical_ring_stop(struct intel_engine_cs *engine); void intel_logical_ring_cleanup(struct intel_engine_cs *engine); int logical_render_ring_init(struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 00ff572541b6..69ccf4fd22f0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2103,7 +2103,7 @@ void intel_legacy_submission_resume(struct drm_i915_private *dev_priv) } } -int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request) +static int ring_request_alloc(struct drm_i915_gem_request *request) { int ret; @@ -2598,6 +2598,8 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv, engine->context_pin = intel_ring_context_pin; engine->context_unpin = intel_ring_context_unpin; + engine->request_alloc = ring_request_alloc; + engine->emit_breadcrumb = i9xx_emit_breadcrumb; engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz; if (i915.semaphores) { diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 4f1271821fa9..0969de7d5ab7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -270,6 +270,7 @@ struct intel_engine_cs { struct i915_gem_context *ctx); void (*context_unpin)(struct intel_engine_cs *engine, struct i915_gem_context *ctx); + int (*request_alloc)(struct drm_i915_gem_request *req); int (*init_context)(struct drm_i915_gem_request *req); int (*emit_flush)(struct drm_i915_gem_request *request, @@ -491,8 +492,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine); void intel_legacy_submission_resume(struct drm_i915_private *dev_priv); -int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request); - int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n); int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req); -- cgit From d3ef1af6fdb67705d4569ac4b6ee11d91f6ab794 Mon Sep 17 00:00:00 2001 From: Daniele Ceraolo Spurio Date: Fri, 23 Dec 2016 15:56:21 -0800 Subject: drm/i915: request ring to be pinned above GUC_WOPCM_TOP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GuC will validate the ring offset and fail if it is in the [0, GUC_WOPCM_TOP) range. The bias is conditionally applied only if GuC loading is enabled (we can't check for guc submission enabled as in other cases because HuC loading requires this fix). Note that the default context is processed before enable_guc_loading is sanitized, so we might still apply the bias to its ring even if it is not needed. v2: compute the value during ctx init and pass it to intel_ring_pin (Chris), updated commit message Signed-off-by: Daniele Ceraolo Spurio Cc: Chris Wilson Cc: Michal Wajdeczko Cc: Arkadiusz Hiler Cc: Anusha Srivatsa Cc: Michał Winiarski Link: http://patchwork.freedesktop.org/patch/msgid/1482537382-28584-1-git-send-email-daniele.ceraolospurio@intel.com Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_context.c | 9 +++++++++ drivers/gpu/drm/i915/intel_lrc.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++++++---- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 5 files changed, 19 insertions(+), 6 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.h') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 32e2d7431025..5194686ea524 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1087,6 +1087,7 @@ struct i915_gem_context { int priority; /* greater priorities are serviced first */ u32 ggtt_alignment; + u32 ggtt_offset_bias; struct intel_context { struct i915_vma *state; diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 15b25c1b1f4d..48e4ed5bb209 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -335,6 +335,15 @@ __create_hw_context(struct drm_i915_private *dev_priv, GEN8_CTX_ADDRESSING_MODE_SHIFT; ATOMIC_INIT_NOTIFIER_HEAD(&ctx->status_notifier); + /* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not + * present or not in use we still need a small bias as ring wraparound + * at offset 0 sometimes hangs. No idea why. + */ + if (HAS_GUC(dev_priv) && i915.enable_guc_loading) + ctx->ggtt_offset_bias = GUC_WOPCM_TOP; + else + ctx->ggtt_offset_bias = 4096; + return ctx; err_pid: diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d322d3c11201..cec9037baf65 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -796,7 +796,7 @@ static int execlists_context_pin(struct intel_engine_cs *engine, goto unpin_vma; } - ret = intel_ring_pin(ce->ring); + ret = intel_ring_pin(ce->ring, ctx->ggtt_offset_bias); if (ret) goto unpin_map; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 69ccf4fd22f0..e84d488e167a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1805,10 +1805,9 @@ static int init_phys_status_page(struct intel_engine_cs *engine) return 0; } -int intel_ring_pin(struct intel_ring *ring) +int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias) { - /* Ring wraparound at offset 0 sometimes hangs. No idea why. */ - unsigned int flags = PIN_GLOBAL | PIN_OFFSET_BIAS | 4096; + unsigned int flags; enum i915_map_type map; struct i915_vma *vma = ring->vma; void *addr; @@ -1818,6 +1817,9 @@ int intel_ring_pin(struct intel_ring *ring) map = HAS_LLC(ring->engine->i915) ? I915_MAP_WB : I915_MAP_WC; + flags = PIN_GLOBAL; + if (offset_bias) + flags |= PIN_OFFSET_BIAS | offset_bias; if (vma->obj->stolen) flags |= PIN_MAPPABLE; @@ -2046,7 +2048,8 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine) goto error; } - ret = intel_ring_pin(ring); + /* Ring wraparound at offset 0 sometimes hangs. No idea why. */ + ret = intel_ring_pin(ring, 4096); if (ret) { intel_ring_free(ring); goto error; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 0969de7d5ab7..79c2b8d72322 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -483,7 +483,7 @@ intel_write_status_page(struct intel_engine_cs *engine, struct intel_ring * intel_engine_create_ring(struct intel_engine_cs *engine, int size); -int intel_ring_pin(struct intel_ring *ring); +int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias); void intel_ring_unpin(struct intel_ring *ring); void intel_ring_free(struct intel_ring *ring); -- cgit