On Mon, Jun 19, 2017 at 12:53 PM, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> Quoting Kenneth Graunke (2017-06-19 20:28:31) > > On Monday, June 19, 2017 3:06:48 AM PDT Chris Wilson wrote: > > > - if (target != batch->bo) > > > - add_exec_bo(batch, target); > > > + if (target != batch->bo) { > > > + unsigned int index = add_exec_bo(batch, target); > > > + struct drm_i915_gem_exec_object2 *exec = > &batch->exec_objects[index]; > > > + > > > + if (write_domain) { > > > + exec->flags |= EXEC_OBJECT_WRITE; > > > + > > > + /* PIPECONTROL needs a w/a on gen6 */ > > > + if (write_domain == I915_GEM_DOMAIN_INSTRUCTION) { > > > + struct brw_context *brw = container_of(batch, brw, batch); > > > + if (brw->gen == 6) > > > + exec->flags |= EXEC_OBJECT_NEEDS_GTT; > > > + } > > > + } > > > + > > > + offset64 = exec->offset; > > > > I don't think this works. brw_bo still has a single offset64 value > that's > > shared across all contexts. So, in a multi-context/multi-threaded case, > we > > may put a bogus offset in the validation list. > > But the value that we put there is the *only* value that we then use for > all of the relocation entries. We are consistently wrong which is what > is mandated by the interface, i.e. we don't prevent a relocation stall > (unless the kernel can move the buffers for us) but more importantly the > relocation.presumed_offset == execobject.offset == batch[reloc.offset] > for all relocations. > Is the BO address in the kernel per-context or per-drm-fd? If it's the later then we may be safe though it sounds scary. If it's the former, then we definitely have a problem. > > Pulling the value out of the validation list is nice, but it's still > > potentially bogus. > > > > I think we still need per-context-bo fields. I have a patch series in > > progress to do that, but I moved it down the priority list a ways... > > need to get back to that... > > Per-context tracking will allow us to avoid stalls between contexts, > which will be an improvement above and beyond this patch. This patch > will work even with conflicting contexts, just no better than today. For > the single context (or no conflict cases), we do realise the improvement > from avoiding always having to inspect reloc[]. > > Hope that helps, > -Chris > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev