[Mesa-dev] [PATCH 2/4] i965: Use I915_EXEC_NO_RELOC

[Jason Ekstrand]

On Mon, Jun 19, 2017 at 12:53 PM, Chris Wilson <chris at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170619/2d2e2131/attachment.html>