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

[Chris Wilson]

Quoting Jason Ekstrand (2017-06-19 22:00:45)
> 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.

The binding of a buffer into its ppGTT is local to that context. The
kernel allocates an address for a buffer separately for each context.
(At the start, the motivating factor was to keep the shared GGTT with
its mappable/non-mappable restrictions entirely separate from ppGTT).
One trick the kernel finally just learnt is to use the supplied offset
for new buffers if it is valid and available. That reduces the impact of
using an offset from context1 in context2, so long as that address was
not already allocated to something else. (So hit and miss in a long
running clients, but any hit is better than always missing.)

However, with softpin I suggest mesa allocates an address at the fd
level and assign that for all contexts using the bo. It does
dramatically reduce the working space, but it still should be many times
physical, except for the complications with 32b restrictions. It was
definitely a very simple proof-of-concept patch (device level monotonic
allocator, falling back to kernel on exhaustion). 
-Chris