Quoting Tvrtko Ursulin (2018-09-14 11:03:18) > > On 14/09/2018 10:51, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-09-14 10:43:12) > >> > >> On 14/09/2018 10:21, Chris Wilson wrote: > >>> Check we can indeed acquire a WB mapping of the context image on module > >>> load. Later this will give us the opportunity to validate that we can > >>> switch from WC to WB as required. > >>> > >>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > >>> --- > >>> drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++ > >>> 1 file changed, 11 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >>> index be9d012d851b..37353afec66e 100644 > >>> --- a/drivers/gpu/drm/i915/i915_gem.c > >>> +++ b/drivers/gpu/drm/i915/i915_gem.c > >>> @@ -5424,6 +5424,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) > >>> > >>> for_each_engine(engine, i915, id) { > >>> struct i915_vma *state; > >>> + void *vaddr; > >>> > >>> state = to_intel_context(ctx, engine)->state; > >>> if (!state) > >>> @@ -5446,6 +5447,16 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) > >>> goto err_active; > >>> > >>> engine->default_state = i915_gem_object_get(state->obj); > >>> + > >>> + /* Check we can acquire the image of the context state */ > >>> + vaddr = i915_gem_object_pin_map(engine->default_state, > >>> + I915_MAP_WB); > >>> + if (IS_ERR(vaddr)) { > >>> + err = PTR_ERR(vaddr); > >>> + goto err_active; > >>> + } > >>> + > >>> + i915_gem_object_unpin_map(engine->default_state); > >> > >> Ah this perhaps strengthens the argument to have > >> __intel_engines_pin_default_state? (And the unpin pair.) > > > > I hope it doesn't come to making this into an interface :) > > Definitely not an idea I endorse, random manipulation of the > > default_state. > > Double underscore alleviates those concerns, no? :) Bah, did you see the EXPORT_SYMBOL_GPL(__i915_gem_object_set_pages), nothing is safe! > Would makes it only one place the following patch needed to switch to > MAP_FORCE_WB. But that was the point of having a separate step here. This is then the only mapping of default_state that needs to do the force. I liked that. The other one we need is the initialisation of the lrc state. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx