On Thu, Nov 24, 2016 at 01:45:12PM +0200, Joonas Lahtinen wrote:
> On ke, 2016-11-23 at 14:11 +0000, Chris Wilson wrote:
> > Soft-pinning depends upon being able to check for availabilty of an
> > interval and evict overlapping object from a drm_mm range manager very
> > quickly. Currently it uses a linear list, and so performance is dire and
> > not suitable as a general replacement. Worse, the current code will oops
> > if it tries to evict an active buffer.
> >
> > It also helps if the routine reports the correct error codes as expected
> > by its callers and emits a tracepoint upon use.
> >
> > For posterity since the wrong patch was pushed (i.e. that missed these
> > key points and had known bugs), this is the changelog that should have
> > been on commit 506a8e87d8d2 ("drm/i915: Add soft-pinning API for
> > execbuffer"):
> >
> > Userspace can pass in an offset that it presumes the object is located
> > at. The kernel will then do its utmost to fit the object into that
> > location. The assumption is that userspace is handling its own object
> > locations (for example along with full-ppgtt) and that the kernel will
> > rarely have to make space for the user's requests.
> >
> > This extends the DRM_IOCTL_I915_GEM_EXECBUFFER2 to do the following:
> > * if the user supplies a virtual address via the execobject->offset
> > *and* sets the EXEC_OBJECT_PINNED flag in execobject->flags, then
> > that object is placed at that offset in the address space selected
> > by the context specifier in execbuffer.
> > * the location must be aligned to the GTT page size, 4096 bytes
> > * as the object is placed exactly as specified, it may be used by this
> > execbuffer call without relocations pointing to it
> >
> > It may fail to do so if:
> > * EINVAL is returned if the object does not have a 4096 byte aligned
> > address
> > * the object conflicts with another pinned object (either pinned by
> > hardware in that address space, e.g. scanouts in the aliasing ppgtt)
> > or within the same batch.
> > EBUSY is returned if the location is pinned by hardware
> > EINVAL is returned if the location is already in use by the batch
> > * EINVAL is returned if the object conflicts with its own alignment (as meets
> > the hardware requirements) or if the placement of the object does not fit
> > within the address space
> >
> > All other execbuffer errors apply.
> >
> > Presence of this execbuf extension may be queried by passing
> > I915_PARAM_HAS_EXEC_SOFTPIN to DRM_IOCTL_I915_GETPARAM and checking for
> > a reported value of 1 (or greater).
> >
> > v2: Combine the hole/adjusted-hole ENOSPC checks
> > v3: More color, more splitting, more blurb.
> >
> > Fixes: 506a8e87d8d2 ("drm/i915: Add soft-pinning API for execbuffer")
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
> <SNIP>
>
> > +TRACE_EVENT(i915_gem_evict_vma,
> > + TP_PROTO(struct i915_vma *vma, unsigned int flags),
> > + TP_ARGS(vma, flags),
> > +
> > + TP_STRUCT__entry(
> > + __field(u32, dev)
> > + __field(struct i915_address_space *, vm)
> > + __field(u64, start)
> > + __field(u64, size)
> > + __field(unsigned int, flags)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->dev = vma->vm->dev->primary->index;
> > + __entry->vm = vma->vm;
> > + __entry->start = vma->node.start;
> > + __entry->size = vma->node.size;
> > + __entry->flags = flags;
> > + ),
> > +
> > + TP_printk("dev=%d, vm=%p, start=%llx size=%llx, flags=%x", __entry->dev, __entry->vm, (long long)__entry->start, (long long)__entry->size, __entry->flags)
>
> A few newlines could be useful. (long long) explicit conversion is not
> done elsewhere in the func but should not hurt. Can't complain on lack
> of consistency as it's wild already :P
The (long long) is wrong. u64 should be %llx. Can't remember if there is
a good excuse or not (it was almost certainly copied from elsewhere ;)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
