Commit 430ffaf4 authored by Chris Wilson's avatar Chris Wilson Committed by Jani Nikula

drm/i915: Suppress switch_mm emission between the same aliasing_ppgtt

When switching between contexts using the aliasing_ppgtt, the VM is
shared. We don't need to reload the PD registers unless they are dirty.

Martin Peres reported an issue that looks like corruption between
Haswell context switches, bisecting to commit f9326be5 ("drm/i915:
Rearrange switch_context to load the aliasing ppgtt on first use").
Switching between the same mm (the aliasing_ppgtt is used for all
contexts in this case) should be a nop, but appears to trigger some
side-effects in the context switch. However, as we know the switch
is redundant in this case, we can skip it and continue to ignore the
issue until somebody feels strong enough to investigate full-ppgtt on
gen7 again!

Except.. Martin was using full-ppgtt which is not supported as it
doesn't work correctly yet. So whilst the bisect did yield valuable
information about the failures, the fix should not have any user impact
under default settings, with the exception of a slightly lower
throughput on xcs as the VM would always be reloaded.

v2: Also remember to set the legacy_active_context following the switch
on xcs (commit e8a9c58f ("drm/i915: Unify active context tracking
between legacy/execlists/guc"))

Fixes: f9326be5 ("drm/i915: Rearrange switch_context to load the aliasing ppgtt on first use")
Fixes: e8a9c58f ("drm/i915: Unify active context tracking between legacy/execlists/guc")
Reported-by: default avatarMartin Peres <martin.peres@linux.intel.com>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Martin Peres <martin.peres@linux.intel.com>
Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170812152724.6883-1-chris@chris-wilson.co.uk
(cherry picked from commit 12124bea)
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 7eceb9d0
...@@ -688,19 +688,19 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt, ...@@ -688,19 +688,19 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt *ppgtt,
} }
static bool static bool
needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt, needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt, struct intel_engine_cs *engine)
struct intel_engine_cs *engine,
struct i915_gem_context *to)
{ {
struct i915_gem_context *from = engine->legacy_active_context;
if (!ppgtt) if (!ppgtt)
return false; return false;
/* Always load the ppgtt on first use */ /* Always load the ppgtt on first use */
if (!engine->legacy_active_context) if (!from)
return true; return true;
/* Same context without new entries, skip */ /* Same context without new entries, skip */
if (engine->legacy_active_context == to && if ((!from->ppgtt || from->ppgtt == ppgtt) &&
!(intel_engine_flag(engine) & ppgtt->pd_dirty_rings)) !(intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
return false; return false;
...@@ -744,7 +744,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) ...@@ -744,7 +744,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
if (skip_rcs_switch(ppgtt, engine, to)) if (skip_rcs_switch(ppgtt, engine, to))
return 0; return 0;
if (needs_pd_load_pre(ppgtt, engine, to)) { if (needs_pd_load_pre(ppgtt, engine)) {
/* Older GENs and non render rings still want the load first, /* Older GENs and non render rings still want the load first,
* "PP_DCLV followed by PP_DIR_BASE register through Load * "PP_DCLV followed by PP_DIR_BASE register through Load
* Register Immediate commands in Ring Buffer before submitting * Register Immediate commands in Ring Buffer before submitting
...@@ -841,7 +841,7 @@ int i915_switch_context(struct drm_i915_gem_request *req) ...@@ -841,7 +841,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
struct i915_hw_ppgtt *ppgtt = struct i915_hw_ppgtt *ppgtt =
to->ppgtt ?: req->i915->mm.aliasing_ppgtt; to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
if (needs_pd_load_pre(ppgtt, engine, to)) { if (needs_pd_load_pre(ppgtt, engine)) {
int ret; int ret;
trace_switch_mm(engine, to); trace_switch_mm(engine, to);
...@@ -852,6 +852,7 @@ int i915_switch_context(struct drm_i915_gem_request *req) ...@@ -852,6 +852,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine);
} }
engine->legacy_active_context = to;
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment