[Intel-gfx] [PATCH 44/62] drm/i915: Prepare i915_gem_active for annotations
Chris Wilson
chris at chris-wilson.co.uk
Fri Jun 3 16:37:09 UTC 2016
In the future, we will want to add annotations to the i915_gem_active
struct. The API is thus expanded to hide direct access to the contents
of i915_gem_active and mediated instead through a number of helpers.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 13 ++--
drivers/gpu/drm/i915/i915_gem.c | 91 +++++++++++++----------
drivers/gpu/drm/i915/i915_gem_fence.c | 11 ++-
drivers/gpu/drm/i915/i915_gem_request.h | 128 +++++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
drivers/gpu/drm/i915/i915_gem_userptr.c | 8 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 9 ++-
drivers/gpu/drm/i915/intel_display.c | 12 ++-
8 files changed, 206 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2edbf9e95e7f..fefb35c4becc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -155,10 +155,10 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
obj->base.write_domain);
for_each_engine_id(engine, dev_priv, id)
seq_printf(m, "%x ",
- i915_gem_request_get_seqno(obj->last_read[id].request));
+ i915_gem_active_get_seqno(&obj->last_read[id]));
seq_printf(m, "] %x %x%s%s%s",
- i915_gem_request_get_seqno(obj->last_write.request),
- i915_gem_request_get_seqno(obj->last_fence.request),
+ i915_gem_active_get_seqno(&obj->last_write),
+ i915_gem_active_get_seqno(&obj->last_fence),
i915_cache_level_str(to_i915(obj->base.dev), obj->cache_level),
obj->dirty ? " dirty" : "",
obj->madv == I915_MADV_DONTNEED ? " purgeable" : "");
@@ -192,8 +192,11 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
*t = '\0';
seq_printf(m, " (%s mappable)", s);
}
- if (obj->last_write.request != NULL)
- seq_printf(m, " (%s)", obj->last_write.request->engine->name);
+
+ engine = i915_gem_active_get_engine(&obj->last_write);
+ if (engine)
+ seq_printf(m, " (%s)", engine->name);
+
if (obj->frontbuffer_bits)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8c3b39a8e974..99e3b269b4b9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1113,29 +1113,32 @@ int
i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
bool readonly)
{
+ struct drm_i915_gem_request *request;
int ret, i;
if (!obj->active)
return 0;
if (readonly) {
- if (obj->last_write.request != NULL) {
- ret = i915_wait_request(obj->last_write.request);
+ request = i915_gem_active_peek(&obj->last_write);
+ if (request) {
+ ret = i915_wait_request(request);
if (ret)
return ret;
- i = obj->last_write.request->engine->id;
- if (obj->last_read[i].request == obj->last_write.request)
+ i = request->engine->id;
+ if (i915_gem_active_peek(&obj->last_read[i]) == request)
i915_gem_object_retire__read(obj, i);
else
i915_gem_object_retire__write(obj);
}
} else {
for (i = 0; i < I915_NUM_ENGINES; i++) {
- if (obj->last_read[i].request == NULL)
+ request = i915_gem_active_peek(&obj->last_read[i]);
+ if (!request)
continue;
- ret = i915_wait_request(obj->last_read[i].request);
+ ret = i915_wait_request(request);
if (ret)
return ret;
@@ -1153,9 +1156,9 @@ i915_gem_object_retire_request(struct drm_i915_gem_object *obj,
{
int ring = req->engine->id;
- if (obj->last_read[ring].request == req)
+ if (i915_gem_active_peek(&obj->last_read[ring]) == req)
i915_gem_object_retire__read(obj, ring);
- else if (obj->last_write.request == req)
+ else if (i915_gem_active_peek(&obj->last_write) == req)
i915_gem_object_retire__write(obj);
if (req->reset_counter == i915_reset_counter(&req->i915->gpu_error))
@@ -1184,20 +1187,20 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
if (readonly) {
struct drm_i915_gem_request *req;
- req = obj->last_write.request;
+ req = i915_gem_active_peek(&obj->last_write);
if (req == NULL)
return 0;
- requests[n++] = i915_gem_request_get(req);
+ requests[n++] = req;
} else {
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct drm_i915_gem_request *req;
- req = obj->last_read[i].request;
+ req = i915_gem_active_peek(&obj->last_read[i]);
if (req == NULL)
continue;
- requests[n++] = i915_gem_request_get(req);
+ requests[n++] = req;
}
}
@@ -2117,25 +2120,27 @@ void i915_vma_move_to_active(struct i915_vma *vma,
static void
i915_gem_object_retire__write(struct drm_i915_gem_object *obj)
{
- GEM_BUG_ON(obj->last_write.request == NULL);
- GEM_BUG_ON(!(obj->active & intel_engine_flag(obj->last_write.request->engine)));
+ GEM_BUG_ON(!__i915_gem_active_is_busy(&obj->last_write));
+ GEM_BUG_ON(!(obj->active & intel_engine_flag(i915_gem_active_get_engine(&obj->last_write))));
- i915_gem_request_assign(&obj->last_write.request, NULL);
+ i915_gem_active_set(&obj->last_write, NULL);
intel_fb_obj_flush(obj, true, ORIGIN_CS);
}
static void
i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
{
+ struct intel_engine_cs *engine;
struct i915_vma *vma;
- GEM_BUG_ON(obj->last_read[ring].request == NULL);
+ GEM_BUG_ON(!__i915_gem_active_is_busy(&obj->last_read[ring]));
GEM_BUG_ON(!(obj->active & (1 << ring)));
list_del_init(&obj->engine_list[ring]);
- i915_gem_request_assign(&obj->last_read[ring].request, NULL);
+ i915_gem_active_set(&obj->last_read[ring], NULL);
- if (obj->last_write.request && obj->last_write.request->engine->id == ring)
+ engine = i915_gem_active_get_engine(&obj->last_write);
+ if (engine && engine->id == ring)
i915_gem_object_retire__write(obj);
obj->active &= ~(1 << ring);
@@ -2154,7 +2159,7 @@ i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
}
- i915_gem_request_assign(&obj->last_fence.request, NULL);
+ i915_gem_active_set(&obj->last_fence, NULL);
i915_gem_object_put(obj);
}
@@ -2347,7 +2352,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *engine)
struct drm_i915_gem_object,
engine_list[engine->id]);
- if (!list_empty(&obj->last_read[engine->id].request->list))
+ if (!list_empty(&i915_gem_active_peek(&obj->last_read[engine->id])->list))
break;
i915_gem_object_retire__read(obj, engine->id);
@@ -2453,7 +2458,7 @@ i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct drm_i915_gem_request *req;
- req = obj->last_read[i].request;
+ req = i915_gem_active_peek(&obj->last_read[i]);
if (req == NULL)
continue;
@@ -2491,7 +2496,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
{
struct drm_i915_gem_wait *args = data;
struct drm_i915_gem_object *obj;
- struct drm_i915_gem_request *req[I915_NUM_ENGINES];
+ struct drm_i915_gem_request *requests[I915_NUM_ENGINES];
int i, n = 0;
int ret;
@@ -2527,20 +2532,21 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
i915_gem_object_put(obj);
for (i = 0; i < I915_NUM_ENGINES; i++) {
- if (obj->last_read[i].request == NULL)
- continue;
+ struct drm_i915_gem_request *req;
- req[n++] = i915_gem_request_get(obj->last_read[i].request);
+ req = i915_gem_active_get(&obj->last_read[i]);
+ if (req)
+ requests[n++] = req;
}
mutex_unlock(&dev->struct_mutex);
for (i = 0; i < n; i++) {
if (ret == 0)
- ret = __i915_wait_request(req[i], true,
+ ret = __i915_wait_request(requests[i], true,
args->timeout_ns > 0 ? &args->timeout_ns : NULL,
to_rps_client(file));
- i915_gem_request_put(req[i]);
+ i915_gem_request_put(requests[i]);
}
return ret;
@@ -2613,7 +2619,7 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
struct drm_i915_gem_request *to)
{
const bool readonly = obj->base.pending_write_domain == 0;
- struct drm_i915_gem_request *req[I915_NUM_ENGINES];
+ struct drm_i915_gem_request *requests[I915_NUM_ENGINES];
int ret, i, n;
if (!obj->active)
@@ -2621,15 +2627,22 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
n = 0;
if (readonly) {
- if (obj->last_write.request)
- req[n++] = obj->last_write.request;
+ struct drm_i915_gem_request *req;
+
+ req = i915_gem_active_peek(&obj->last_write);
+ if (req)
+ requests[n++] = req;
} else {
- for (i = 0; i < I915_NUM_ENGINES; i++)
- if (obj->last_read[i].request)
- req[n++] = obj->last_read[i].request;
+ for (i = 0; i < I915_NUM_ENGINES; i++) {
+ struct drm_i915_gem_request *req;
+
+ req = i915_gem_active_peek(&obj->last_read[i]);
+ if (req)
+ requests[n++] = req;
+ }
}
for (i = 0; i < n; i++) {
- ret = __i915_gem_object_sync(obj, to, req[i]);
+ ret = __i915_gem_object_sync(obj, to, requests[i]);
if (ret)
return ret;
}
@@ -3702,17 +3715,17 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
args->busy = 0;
if (obj->active) {
+ struct drm_i915_gem_request *req;
int i;
for (i = 0; i < I915_NUM_ENGINES; i++) {
- struct drm_i915_gem_request *req;
-
- req = obj->last_read[i].request;
+ req = i915_gem_active_peek(&obj->last_read[i]);
if (req)
args->busy |= 1 << (16 + req->engine->exec_id);
}
- if (obj->last_write.request)
- args->busy |= obj->last_write.request->engine->exec_id;
+ req = i915_gem_active_peek(&obj->last_write);
+ if (req)
+ args->busy |= req->engine->exec_id;
}
unref:
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c
index 9f8ce13d2f77..301344252b18 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -261,14 +261,13 @@ static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj)
static int
i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)
{
- if (obj->last_fence.request) {
- int ret = i915_wait_request(obj->last_fence.request);
- if (ret)
- return ret;
+ int ret;
- i915_gem_request_assign(&obj->last_fence.request, NULL);
- }
+ ret = i915_gem_active_wait(&obj->last_fence);
+ if (ret)
+ return ret;
+ i915_gem_active_set(&obj->last_fence, NULL);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index 89d7bb651f67..56e312b95407 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -271,14 +271,138 @@ static inline bool i915_spin_request(const struct drm_i915_gem_request *request,
* resource including itself.
*/
struct i915_gem_active {
- struct drm_i915_gem_request *request;
+ struct drm_i915_gem_request *__request;
};
+/**
+ * i915_gem_active_set - updates the tracker to watch the current request
+ * @active - the active tracker
+ * @request - the request to watch
+ *
+ * i915_gem_active_set() watches the given @request for completion. Whilst
+ * that @request is busy, the @active reports busy. When that @request is
+ * retired, the @active tracker is updated to report idle.
+ */
static inline void
i915_gem_active_set(struct i915_gem_active *active,
struct drm_i915_gem_request *request)
{
- i915_gem_request_assign(&active->request, request);
+ i915_gem_request_assign(&active->__request, request);
+}
+
+/**
+ * i915_gem_active_peek - report the request being monitored
+ * @active - the active tracker
+ *
+ * i915_gem_active_peek() returns the current request being tracked, or NULL.
+ * It does not obtain a reference on the request for the caller, so the
+ * caller must hold struct_mutex.
+ */
+static inline struct drm_i915_gem_request *
+i915_gem_active_peek(const struct i915_gem_active *active)
+{
+ return active->__request;
+}
+
+/**
+ * i915_gem_active_get - return a reference to the active request
+ * @active - the active tracker
+ *
+ * i915_gem_active_get() returns a reference to the active request, or NULL
+ * if the active tracker is idle. The caller must hold struct_mutex.
+ */
+static inline struct drm_i915_gem_request *
+i915_gem_active_get(const struct i915_gem_active *active)
+{
+ struct drm_i915_gem_request *request;
+
+ request = i915_gem_active_peek(active);
+ if (!request || i915_gem_request_completed(request))
+ return NULL;
+
+ return i915_gem_request_get(request);
+}
+
+/**
+ * __i915_gem_active_is_busy - report whether the active tracker is assigned
+ * @active - the active tracker
+ *
+ * __i915_gem_active_is_busy() returns true if the active tracker is currently
+ * assigned to a request. Due to the lazy retiring, that request may be idle
+ * and this may report stale information.
+ */
+static inline bool
+__i915_gem_active_is_busy(const struct i915_gem_active *active)
+{
+ return i915_gem_active_peek(active);
+}
+
+/**
+ * i915_gem_active_is_idle - report whether the active tracker is idle
+ * @active - the active tracker
+ *
+ * i915_gem_active_is_idle() returns true if the active tracker is currently
+ * unassigned or if the request is complete (but not yet retired). Requires
+ * the caller to hold struct_mutex (but that can be relaxed if desired).
+ */
+static inline bool
+i915_gem_active_is_idle(const struct i915_gem_active *active)
+{
+ struct drm_i915_gem_request *request;
+
+ request = i915_gem_active_peek(active);
+ if (!request || i915_gem_request_completed(request))
+ return true;
+
+ return false;
+}
+
+/**
+ * i915_gem_active_wait - waits until the request is completed
+ * @active - the active request on which to wait
+ *
+ * i915_gem_active_wait() waits until the request is completed before
+ * returning.
+ */
+static inline int __must_check
+i915_gem_active_wait(const struct i915_gem_active *active)
+{
+ struct drm_i915_gem_request *request;
+
+ request = i915_gem_active_peek(active);
+ if (!request)
+ return 0;
+
+ return i915_wait_request(request);
+}
+
+/**
+ * i915_gem_active_retire - waits until the request is retired
+ * @active - the active request on which to wait
+ *
+ * Unlike i915_gem_active_eait(), this i915_gem_active_retire() will
+ * make sure the request is retired before returning.
+ */
+static inline int __must_check
+i915_gem_active_retire(const struct i915_gem_active *active)
+{
+ return i915_gem_active_wait(active);
+}
+
+/* Convenience functions for peeking at state inside active's request whilst
+ * guarded by the struct_mutex.
+ */
+
+static inline uint32_t
+i915_gem_active_get_seqno(const struct i915_gem_active *active)
+{
+ return i915_gem_request_get_seqno(i915_gem_active_peek(active));
+}
+
+static inline struct intel_engine_cs *
+i915_gem_active_get_engine(const struct i915_gem_active *active)
+{
+ return i915_gem_request_get_engine(i915_gem_active_peek(active));
}
#define for_each_active(mask, idx) \
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index fc78f49c5815..9bc824421b66 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -242,7 +242,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
}
obj->fence_dirty =
- obj->last_fence.request ||
+ !i915_gem_active_is_idle(&obj->last_fence) ||
obj->fence_reg != I915_FENCE_REG_NONE;
obj->tiling_mode = args->tiling_mode;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 93c2dea90a89..d688558606f9 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -74,11 +74,9 @@ static void wait_rendering(struct drm_i915_gem_object *obj)
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct drm_i915_gem_request *req;
- req = obj->last_read[i].request;
- if (req == NULL)
- continue;
-
- requests[n++] = i915_gem_request_get(req);
+ req = i915_gem_active_get(&obj->last_read[i]);
+ if (req)
+ requests[n++] = req;
}
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index e68718265619..1abcf316a825 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -744,13 +744,14 @@ static void capture_bo(struct drm_i915_error_buffer *err,
struct i915_vma *vma)
{
struct drm_i915_gem_object *obj = vma->obj;
+ struct intel_engine_cs *engine;
int i;
err->size = obj->base.size;
err->name = obj->base.name;
for (i = 0; i < I915_NUM_ENGINES; i++)
- err->rseqno[i] = i915_gem_request_get_seqno(obj->last_read[i].request);
- err->wseqno = i915_gem_request_get_seqno(obj->last_write.request);
+ err->rseqno[i] = i915_gem_active_get_seqno(&obj->last_read[i]);
+ err->wseqno = i915_gem_active_get_seqno(&obj->last_write);
err->gtt_offset = vma->node.start;
err->read_domains = obj->base.read_domains;
err->write_domain = obj->base.write_domain;
@@ -762,8 +763,10 @@ static void capture_bo(struct drm_i915_error_buffer *err,
err->dirty = obj->dirty;
err->purgeable = obj->madv != I915_MADV_WILLNEED;
err->userptr = obj->userptr.mm != NULL;
- err->ring = obj->last_write.request ? obj->last_write.request->engine->id : -1;
err->cache_level = obj->cache_level;
+
+ engine = i915_gem_active_get_engine(&obj->last_write);
+ err->ring = engine ? engine->id : -1;
}
static u32 capture_active_bo(struct drm_i915_error_buffer *err,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92e7bc76cce0..839c46a007b5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11427,7 +11427,7 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
false))
return true;
else
- return engine != i915_gem_request_get_engine(obj->last_write.request);
+ return engine != i915_gem_active_get_engine(&obj->last_write);
}
static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
@@ -11727,7 +11727,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
engine = &dev_priv->engine[BCS];
} else if (INTEL_INFO(dev)->gen >= 7) {
- engine = i915_gem_request_get_engine(obj->last_write.request);
+ engine = i915_gem_active_get_engine(&obj->last_write);
if (engine == NULL || engine->id != RCS)
engine = &dev_priv->engine[BCS];
} else {
@@ -11748,9 +11748,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (mmio_flip) {
INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
- i915_gem_request_assign(&work->flip_queued_req,
- obj->last_write.request);
-
+ work->flip_queued_req = i915_gem_active_get(&obj->last_write);
schedule_work(&work->mmio_work);
} else {
request = i915_gem_request_alloc(engine, engine->last_context);
@@ -13971,8 +13969,8 @@ intel_prepare_plane_fb(struct drm_plane *plane,
struct intel_plane_state *plane_state =
to_intel_plane_state(new_state);
- i915_gem_request_assign(&plane_state->wait_req,
- obj->last_write.request);
+ plane_state->wait_req =
+ i915_gem_active_get(&obj->last_write);
}
i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
--
2.8.1
More information about the Intel-gfx
mailing list