On 13/05/2016 08:39, Chris Wilson wrote:
On Thu, May 12, 2016 at 10:06:31PM +0100, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
The fence object used inside the request structure requires a sequence
number. Although this is not used by the i915 driver itself, it could
potentially be used by non-i915 code if the fence is passed outside of
the driver. This is the intention as it allows external kernel drivers
and user applications to wait on batch buffer completion
asynchronously via the dma-buff fence API.
To ensure that such external users are not confused by strange things
happening with the seqno, this patch adds in a per context timeline
that can provide a guaranteed in-order seqno value for the fence. This
is safe because the scheduler will not re-order batch buffers within a
context - they are considered to be mutually dependent.
v2: New patch in series.
v3: Renamed/retyped timeline structure fields after review comments by
Tvrtko Ursulin.
Added context information to the timeline's name string for better
identification in debugfs output.
v5: Line wrapping and other white space fixes to keep style checker
happy.
v7: Updated to newer nightly (lots of ring -> engine renaming).
v8: Moved to earlier in patch series so no longer needs to remove the
quick hack timeline that was being added before.
For: VIZ-5190
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 14 ++++++++++++
drivers/gpu/drm/i915/i915_gem.c | 40 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_gem_context.c | 14 ++++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 8 +++++++
4 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d5496ab..520480b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -818,6 +818,19 @@ struct i915_ctx_hang_stats {
bool banned;
};
+struct i915_fence_timeline {
+ char name[32];
+ unsigned fence_context;
+ unsigned next;
+
+ struct intel_context *ctx;
+ struct intel_engine_cs *engine;
+};
+
+int i915_create_fence_timeline(struct drm_device *dev,
+ struct intel_context *ctx,
+ struct intel_engine_cs *ring);
+
/* This must match up with the value previously used for execbuf2.rsvd1. */
#define DEFAULT_CONTEXT_HANDLE 0
@@ -869,6 +882,7 @@ struct intel_context {
u64 lrc_desc;
uint32_t *lrc_reg_state;
bool initialised;
+ struct i915_fence_timeline fence_timeline;
This is still fundamentally wrong. This i915_fence_timeline is both the
fence context and timeline. Our timelines are singular per vm, with a
fence context under each timeline per engine.
As I said last time you mentioned 'per vm', please explain. Where does a
virtual machine come in? Or is that not what you mean? What is the
purpose of having multiple fence contexts within a single timeline? It
will stop external uses attempting to combine fences but it won't stop
them from actually being out of order. The timeline needs to be per
hardware context not simply per engine because the whole point is that
requests should not be re-ordered once they have been allocated a point
on a timeline. However, the purpose of the scheduler (which is what this
series is preparation for) is to re-order requests between contexts.
Thus the timeline must be per context to prevent requests ending up with
out of order completions.
Please complete the legacy/execlists unification first so that we can
have timelines work correctly for the whole driver.
No. That is a much larger task that people are working towards. However,
we urgently need a scheduler for specific customers to use right now.
That means we need to get something working right now not in some random
number of years time. If an intermediate step is less than perfect but
still functional that is better than not having anything at all.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
