Commit 666424ab authored by Chris Wilson's avatar Chris Wilson

drm/i915/execlists: Use coherent writes into the context image

That we use a WB mapping for updating the RING_TAIL register inside the
context image even on !llc machines has been a source of consternation
for every reader. It appears to work on bsw+, but it may just have been
that we have been incredibly bad at detecting the errors.

v2: With extra enthusiasm.
v3: Drop force of map type for pinned default_state as by the time we
pin it, the map type is always WB and doesn't conflict with the earlier
use by ce->state.
v4: Transfer engine->default_state from MAP_WC to MAP_WB on creation so
we do not need the MAP_FORCE littered around the backends
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180914123504.2062-3-chris@chris-wilson.co.uk
parent 37d7c9cc
...@@ -3096,6 +3096,12 @@ enum i915_map_type { ...@@ -3096,6 +3096,12 @@ enum i915_map_type {
I915_MAP_FORCE_WC = I915_MAP_WC | I915_MAP_OVERRIDE, I915_MAP_FORCE_WC = I915_MAP_WC | I915_MAP_OVERRIDE,
}; };
static inline enum i915_map_type
i915_coherent_map_type(struct drm_i915_private *i915)
{
return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
}
/** /**
* i915_gem_object_pin_map - return a contiguous mapping of the entire object * i915_gem_object_pin_map - return a contiguous mapping of the entire object
* @obj: the object to map into kernel address space * @obj: the object to map into kernel address space
......
...@@ -5418,6 +5418,8 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) ...@@ -5418,6 +5418,8 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
struct i915_vma *state; struct i915_vma *state;
void *vaddr; void *vaddr;
GEM_BUG_ON(to_intel_context(ctx, engine)->pin_count);
state = to_intel_context(ctx, engine)->state; state = to_intel_context(ctx, engine)->state;
if (!state) if (!state)
continue; continue;
...@@ -5442,7 +5444,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) ...@@ -5442,7 +5444,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
/* Check we can acquire the image of the context state */ /* Check we can acquire the image of the context state */
vaddr = i915_gem_object_pin_map(engine->default_state, vaddr = i915_gem_object_pin_map(engine->default_state,
I915_MAP_WB); I915_MAP_FORCE_WB);
if (IS_ERR(vaddr)) { if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr); err = PTR_ERR(vaddr);
goto err_active; goto err_active;
......
...@@ -1707,6 +1707,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, ...@@ -1707,6 +1707,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
const struct i915_oa_config *oa_config) const struct i915_oa_config *oa_config)
{ {
struct intel_engine_cs *engine = dev_priv->engine[RCS]; struct intel_engine_cs *engine = dev_priv->engine[RCS];
unsigned int map_type = i915_coherent_map_type(dev_priv);
struct i915_gem_context *ctx; struct i915_gem_context *ctx;
struct i915_request *rq; struct i915_request *rq;
int ret; int ret;
...@@ -1741,7 +1742,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, ...@@ -1741,7 +1742,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
if (!ce->state) if (!ce->state)
continue; continue;
regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB); regs = i915_gem_object_pin_map(ce->state->obj, map_type);
if (IS_ERR(regs)) if (IS_ERR(regs))
return PTR_ERR(regs); return PTR_ERR(regs);
......
...@@ -1294,7 +1294,7 @@ static int __context_pin(struct i915_gem_context *ctx, struct i915_vma *vma) ...@@ -1294,7 +1294,7 @@ static int __context_pin(struct i915_gem_context *ctx, struct i915_vma *vma)
* on an active context (which by nature is already on the GPU). * on an active context (which by nature is already on the GPU).
*/ */
if (!(vma->flags & I915_VMA_GLOBAL_BIND)) { if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
err = i915_gem_object_set_to_gtt_domain(vma->obj, true); err = i915_gem_object_set_to_wc_domain(vma->obj, true);
if (err) if (err)
return err; return err;
} }
...@@ -1322,7 +1322,9 @@ __execlists_context_pin(struct intel_engine_cs *engine, ...@@ -1322,7 +1322,9 @@ __execlists_context_pin(struct intel_engine_cs *engine,
if (ret) if (ret)
goto err; goto err;
vaddr = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB); vaddr = i915_gem_object_pin_map(ce->state->obj,
i915_coherent_map_type(ctx->i915) |
I915_MAP_OVERRIDE);
if (IS_ERR(vaddr)) { if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr); ret = PTR_ERR(vaddr);
goto unpin_vma; goto unpin_vma;
......
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