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
With the newlines, this is;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
