Re: [PATCH v7 3/8] drm/i915: Add per context timelines to fence object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20/04/2016 18:44, Chris Wilson wrote:
On Wed, Apr 20, 2016 at 06:09:50PM +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).

For: VIZ-5190
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h         | 25 +++++++---
  drivers/gpu/drm/i915/i915_gem.c         | 83 +++++++++++++++++++++++++++++----
  drivers/gpu/drm/i915/i915_gem_context.c | 14 ++++++
  drivers/gpu/drm/i915/intel_lrc.c        |  8 ++++
  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
  5 files changed, 114 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bb18b89..1c3a1ca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -813,6 +813,15 @@ 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;
+};
+
  /* This must match up with the value previously used for execbuf2.rsvd1. */
  #define DEFAULT_CONTEXT_HANDLE 0
@@ -860,6 +869,7 @@ struct intel_context {
  		struct i915_vma *lrc_vma;
  		u64 lrc_desc;
  		uint32_t *lrc_reg_state;
+		struct i915_fence_timeline fence_timeline;
This is wrong. The timeline is actually a property of the vm, with
contexts for each engine.
-Chris


I don't get what you mean. The timeline does not have contexts. It is just an incrementing number. It could possible be shared between all engines of a single software context rather than be specific to the hardware context. Although I think it is safer and more future proof to be the latter, i.e. per engine per s/w context. I don't see where the vm comes into it.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux