Re: [RFC] drm/i915: Add sync framework support to execbuff IOCTL

Chris Wilson
2015-07-02T11:54:31Z

On Thu, Jul 02, 2015 at 12:09:59PM +0100, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Various projects desire a mechanism for managing dependencies between
> work items asynchronously. This can also include work items across
> complete different and independent systems. For example, an
> application wants to retreive a frame from a video in device,
> using it for rendering on a GPU then send it to the video out device
> for display all without having to stall waiting for completion along
> the way. The sync framework allows this. It encapsulates
> synchronisation events in file descriptors. The application can
> request a sync point for the completion of each piece of work. Drivers
> should also take sync points in with each new work request and not
> schedule the work to start until the sync has been signalled.
>
> This patch adds sync framework support to the exec buffer IOCTL. A
> sync point can be passed in to stall execution of the batch buffer
> until signalled. And a sync point can be returned after each batch
> buffer submission which will be signalled upon that batch buffer's
> completion.
>
> At present, the input sync point is simply waited on synchronously
> inside the exec buffer IOCTL call. Once the GPU scheduler arrives,
> this will be handled asynchronously inside the scheduler and the IOCTL
> can return without having to wait.
>
> Note also that the scheduler will re-order the execution of batch
> buffers, e.g. because a batch buffer is stalled on a sync point and
> cannot be submitted yet but other, independent, batch buffers are
> being presented to the driver. This means that the timeline within the
> sync points returned cannot be global to the engine. Instead they must
> be kept per context per engine (the scheduler may not re-order batches
> within a context). Hence the timeline cannot be based on the existing
> seqno values but must be a new implementation.

But there is nothing preventing assignment of the sync value on
submission. Other than the debug .fence_value_str it's a private
implementation detail, and the interface is solely through the fd and
signalling. You could implement this as a secondary write to the HWS,
assigning the sync_value to the sync_pt on submission and
remove the request tracking, as when signalled you only need to compare
the sync_value against the timeline value in the HWS.

However, that equally applies to the existing request->seqno. That can
also be assigned on submission so that it always an ordered timeline, and
so can be used internally or externally.

It's a pity that the sync_pt didn't forward the
fence->enable_signaling().

As for using rsvd2, make sure you do

int fence_fd = lower_32_bits(args->rsvd2);

and maybe I915_EXEC_CREATE_FENCE is a clearer name.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre