[Intel-gfx] [RFC 1/4] drm/i915: Convert requests to use struct fence
John.C.Harrison at Intel.com
John.C.Harrison at Intel.com
Fri Mar 20 10:48:34 PDT 2015
From: John Harrison <John.C.Harrison at Intel.com>
There is a construct in the linux kernel called 'struct fence' that is intended
to keep track of work that is executed on hardware. I.e. it solves the basic
problem that the drivers 'struct drm_i915_gem_request' is trying to address. The
request structure does quite a lot more than simply track the execution progress
so is very definitely still required. However, the basic completion status side
could be updated to use the ready made fence implementation and gain all the
advantages that provides.
This patch makes the first step of integrating a struct fence into the request.
It replaces the explicit reference count with that of the fence. It also
replaces the 'is completed' test with the fence's equivalent. Currently, that
simply chains on to the original request implementation. A future patch will
improve this.
For: VIZ-5190
Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 37 +++++++++------------
drivers/gpu/drm/i915/i915_gem.c | 55 ++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_lrc.c | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++
5 files changed, 70 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ce3a536..7dcaf8c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -50,6 +50,7 @@
#include <linux/intel-iommu.h>
#include <linux/kref.h>
#include <linux/pm_qos.h>
+#include <linux/fence.h>
/* General customization:
*/
@@ -2048,7 +2049,11 @@ void i915_gem_track_fb(struct drm_i915_gem_object *old,
* initial reference taken using kref_init
*/
struct drm_i915_gem_request {
- struct kref ref;
+ /** Underlying object for implementing the signal/wait stuff.
+ * NB: Never call fence_later()! Due to lazy allocation, scheduler
+ * re-ordering, pre-emption, etc., there is no guarantee at all
+ * about the validity or sequentialiaty of the fence's seqno! */
+ struct fence fence;
/** On Which ring this request was generated */
struct intel_engine_cs *ring;
@@ -2126,7 +2131,13 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_request **req_out);
void i915_gem_request_cancel(struct drm_i915_gem_request *req);
-void i915_gem_request_free(struct kref *req_ref);
+
+static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
+ bool lazy_coherency)
+{
+ return fence_is_signaled(&req->fence);
+}
+
void i915_gem_request_remove_from_client(struct drm_i915_gem_request *request);
int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
struct drm_file *file);
@@ -2146,14 +2157,14 @@ i915_gem_request_get_ring(struct drm_i915_gem_request *req)
static inline void
i915_gem_request_reference(struct drm_i915_gem_request *req)
{
- kref_get(&req->ref);
+ fence_get(&req->fence);
}
static inline void
i915_gem_request_unreference(struct drm_i915_gem_request *req)
{
WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
- kref_put(&req->ref, i915_gem_request_free);
+ fence_put(&req->fence);
}
static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
@@ -2168,12 +2179,6 @@ static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
*pdst = src;
}
-/*
- * XXX: i915_gem_request_completed should be here but currently needs the
- * definition of i915_seqno_passed() which is below. It will be moved in
- * a later patch when the call to i915_seqno_passed() is obsoleted...
- */
-
struct drm_i915_file_private {
struct drm_i915_private *dev_priv;
struct drm_file *file;
@@ -2691,18 +2696,6 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2)
return (int32_t)(seq1 - seq2) >= 0;
}
-static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
- bool lazy_coherency)
-{
- u32 seqno;
-
- BUG_ON(req == NULL);
-
- seqno = req->ring->get_seqno(req->ring, lazy_coherency);
-
- return i915_seqno_passed(seqno, req->seqno);
-}
-
int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno);
int __must_check i915_gem_set_seqno(struct drm_device *dev, u32 seqno);
int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4eb7fc2..5ede297 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2495,10 +2495,10 @@ static void i915_gem_free_request(struct drm_i915_gem_request *request)
i915_gem_request_unreference(request);
}
-void i915_gem_request_free(struct kref *req_ref)
+static void i915_gem_request_free(struct fence *req_fence)
{
- struct drm_i915_gem_request *req = container_of(req_ref,
- typeof(*req), ref);
+ struct drm_i915_gem_request *req = container_of(req_fence,
+ typeof(*req), fence);
struct intel_context *ctx = req->ctx;
if (req->file_priv)
@@ -2518,6 +2518,46 @@ void i915_gem_request_free(struct kref *req_ref)
kfree(req);
}
+static const char *i915_gem_request_get_driver_name(struct fence *req_fence)
+{
+ return "i915_request";
+}
+
+static const char *i915_gem_request_get_timeline_name(struct fence *req_fence)
+{
+ struct drm_i915_gem_request *req = container_of(req_fence,
+ typeof(*req), fence);
+ return req->ring->name;
+}
+
+static bool i915_gem_request_enable_signaling(struct fence *req_fence)
+{
+ WARN(true, "Is this required?");
+ return true;
+}
+
+static bool i915_gem_request_is_completed(struct fence *req_fence)
+{
+ struct drm_i915_gem_request *req = container_of(req_fence,
+ typeof(*req), fence);
+ u32 seqno;
+
+ BUG_ON(req == NULL);
+
+ seqno = req->ring->get_seqno(req->ring, false/*lazy_coherency*/);
+
+ return i915_seqno_passed(seqno, req->seqno);
+}
+
+static const struct fence_ops i915_gem_request_fops = {
+ .get_driver_name = i915_gem_request_get_driver_name,
+ .get_timeline_name = i915_gem_request_get_timeline_name,
+ .enable_signaling = i915_gem_request_enable_signaling,
+ .signaled = i915_gem_request_is_completed,
+ .wait = fence_default_wait,
+ .release = i915_gem_request_free,
+};
+
int i915_gem_request_alloc(struct intel_engine_cs *ring,
struct intel_context *ctx,
struct drm_i915_gem_request **req_out)
@@ -2541,7 +2581,6 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring,
return ret;
}
- kref_init(&request->ref);
request->ring = ring;
request->uniq = dev_private->request_uniq++;
request->ctx = ctx;
@@ -2557,6 +2596,8 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring,
return ret;
}
+ fence_init(&request->fence, &i915_gem_request_fops, &ring->fence_lock, ring->fence_context, request->seqno);
+
/*
* Reserve space in the ring buffer for all the commands required to
* eventually emit this request. This is to guarantee that the
@@ -4825,7 +4866,7 @@ i915_gem_init_hw(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
- int ret, i;
+ int ret, i, fence_base;
if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
return -EIO;
@@ -4877,12 +4918,16 @@ i915_gem_init_hw(struct drm_device *dev)
goto out;
}
+ fence_base = fence_context_alloc(I915_NUM_RINGS);
+
/* Now it is safe to go back round and do everything else: */
for_each_ring(ring, dev_priv, i) {
struct drm_i915_gem_request *req;
WARN_ON(!ring->default_context);
+ ring->fence_context = fence_base + i;
+
ret = i915_gem_request_alloc(ring, ring->default_context, &req);
if (ret) {
i915_gem_cleanup_ringbuffer(dev);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ae00054..c1072b1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1337,6 +1337,7 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
ring->dev = dev;
INIT_LIST_HEAD(&ring->active_list);
INIT_LIST_HEAD(&ring->request_list);
+ spin_lock_init(&ring->fence_lock);
init_waitqueue_head(&ring->irq_queue);
INIT_LIST_HEAD(&ring->execlist_queue);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6099fce..fd65c0d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1981,6 +1981,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
INIT_LIST_HEAD(&ring->active_list);
INIT_LIST_HEAD(&ring->request_list);
INIT_LIST_HEAD(&ring->execlist_queue);
+ spin_lock_init(&ring->fence_lock);
ringbuf->size = 32 * PAGE_SIZE;
ringbuf->ring = ring;
memset(ring->semaphore.sync_seqno, 0, sizeof(ring->semaphore.sync_seqno));
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 68097c1..a0ce08e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -308,6 +308,9 @@ struct intel_engine_cs {
* to encode the command length in the header).
*/
u32 (*get_cmd_length_mask)(u32 cmd_header);
+
+ unsigned fence_context;
+ spinlock_t fence_lock;
};
bool intel_ring_initialized(struct intel_engine_cs *ring);
--
1.7.9.5
More information about the Intel-gfx
mailing list