On Fri, Sep 14, 2018 at 10:42:15AM +0100, Chris Wilson wrote: > 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.
Presumably it's due to the "all ggtt accesses go through pat[0]" and we make pat[0] snoop. So presumably the hw should snoop when loading the context... maybe. > > 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: Chris Wilson <[email protected]> > Reviewed-by: Tvrtko Ursulin <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ > drivers/gpu/drm/i915/i915_gem.c | 4 +++- > drivers/gpu/drm/i915/i915_perf.c | 3 ++- > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > 4 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index bb43e56df197..7d4daa7412f1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3097,6 +3097,12 @@ enum i915_map_type { > 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 > * @obj: the object to map into kernel address space > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 37353afec66e..d9465bd1a00a 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -5426,6 +5426,8 @@ static int __intel_engines_record_defaults(struct > drm_i915_private *i915) > struct i915_vma *state; > void *vaddr; > > + GEM_BUG_ON(to_intel_context(ctx, engine)->pin_count); > + > state = to_intel_context(ctx, engine)->state; > if (!state) > continue; > @@ -5450,7 +5452,7 @@ static int __intel_engines_record_defaults(struct > drm_i915_private *i915) > > /* Check we can acquire the image of the context state */ > vaddr = i915_gem_object_pin_map(engine->default_state, > - I915_MAP_WB); > + I915_MAP_FORCE_WB); > if (IS_ERR(vaddr)) { > err = PTR_ERR(vaddr); > goto err_active; > diff --git a/drivers/gpu/drm/i915/i915_perf.c > b/drivers/gpu/drm/i915/i915_perf.c > index 3d7a052b4cca..664b96bb65a3 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1707,6 +1707,7 @@ static int gen8_configure_all_contexts(struct > drm_i915_private *dev_priv, > const struct i915_oa_config *oa_config) > { > 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_request *rq; > int ret; > @@ -1741,7 +1742,7 @@ static int gen8_configure_all_contexts(struct > drm_i915_private *dev_priv, > if (!ce->state) > 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)) > return PTR_ERR(regs); > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index d7fcbba8e982..a51be16ddaac 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -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). > */ > 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) > return err; > } > @@ -1322,7 +1322,9 @@ __execlists_context_pin(struct intel_engine_cs *engine, > if (ret) > 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)) { > ret = PTR_ERR(vaddr); > goto unpin_vma; > -- > 2.19.0 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
