[Intel-gfx] [PATCH v4 29/38] drm/i915: Add early exit to execbuff_final() if insufficient ring space
John.C.Harrison at Intel.com
John.C.Harrison at Intel.com
Mon Jan 11 10:42:58 PST 2016
From: John Harrison <John.C.Harrison at Intel.com>
One of the major purposes of the GPU scheduler is to avoid stalling
the CPU when the GPU is busy and unable to accept more work. This
change adds support to the ring submission code to allow a ring space
check to be performed before attempting to submit a batch buffer to
the hardware. If insufficient space is available then the scheduler
can go away and come back later, letting the CPU get on with other
work, rather than stalling and waiting for the hardware to catch up.
v3: Updated to use locally cached request pointer.
v4: Line wrapped some comments differently to keep the style checker
happy. Downgraded a BUG_ON to a WARN_ON as the latter is preferred.
Removed some obsolete, commented out code.
For: VIZ-1587
Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 42 +++++++++++++++++------
drivers/gpu/drm/i915/intel_lrc.c | 54 +++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_ringbuffer.c | 26 ++++++++++++++
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
4 files changed, 108 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index de8607e..6336479 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1099,25 +1099,19 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
{
struct intel_engine_cs *ring = req->ring;
struct drm_i915_private *dev_priv = dev->dev_private;
- int ret, i;
+ int i;
if (!IS_GEN7(dev) || ring != &dev_priv->ring[RCS]) {
DRM_DEBUG("sol reset is gen7/rcs only\n");
return -EINVAL;
}
- ret = intel_ring_begin(req, 4 * 3);
- if (ret)
- return ret;
-
for (i = 0; i < 4; i++) {
intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, GEN7_SO_WRITE_OFFSET(i));
intel_ring_emit(ring, 0);
}
- intel_ring_advance(ring);
-
return 0;
}
@@ -1245,6 +1239,7 @@ int i915_gem_ringbuffer_submission_final(struct i915_execbuffer_params *params)
struct intel_engine_cs *ring = params->ring;
u64 exec_start, exec_len;
int ret;
+ uint32_t min_space;
/* The mutex must be acquired before calling this function */
WARN_ON(!mutex_is_locked(¶ms->dev->struct_mutex));
@@ -1267,8 +1262,36 @@ int i915_gem_ringbuffer_submission_final(struct i915_execbuffer_params *params)
if (ret)
return ret;
+ /*
+ * It would be a bad idea to run out of space while writing commands
+ * to the ring. One of the major aims of the scheduler is to not
+ * stall at any point for any reason. However, doing an early exit
+ * half way through submission could result in a partial sequence
+ * being written which would leave the engine in an unknown state.
+ * Therefore, check in advance that there will be enough space for
+ * the entire submission whether emitted by the code below OR by any
+ * other functions that may be executed before the end of final().
+ *
+ * NB: This test deliberately overestimates, because that's easier
+ * than tracing every potential path that could be taken!
+ *
+ * Current measurements suggest that we may need to emit up to 186
+ * dwords, so this is rounded up to 256 here. Then double that to get
+ * the free space requirement, because the block is not allowed to
+ * span the transition from the end to the beginning of the ring.
+ */
+#define I915_BATCH_EXEC_MAX_LEN 256 /* max dwords emitted here */
+ min_space = I915_BATCH_EXEC_MAX_LEN * 2 * sizeof(uint32_t);
+ ret = intel_ring_test_space(req->ringbuf, min_space);
+ if (ret)
+ goto early_error;
+
intel_runtime_pm_get(dev_priv);
+ ret = intel_ring_begin(req, I915_BATCH_EXEC_MAX_LEN);
+ if (ret)
+ goto error;
+
/*
* Unconditionally invalidate gpu caches and ensure that we do flush
* any residual writes from the previous batch.
@@ -1287,10 +1310,6 @@ int i915_gem_ringbuffer_submission_final(struct i915_execbuffer_params *params)
if (ring == &dev_priv->ring[RCS] &&
params->instp_mode != dev_priv->relative_constants_mode) {
- ret = intel_ring_begin(req, 4);
- if (ret)
- goto error;
-
intel_ring_emit(ring, MI_NOOP);
intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, INSTPM);
@@ -1327,6 +1346,7 @@ error:
*/
intel_runtime_pm_put(dev_priv);
+early_error:
if (ret)
intel_ring_reserved_space_cancel(req->ringbuf);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 92d9b03..ae5dd0a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -226,6 +226,27 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
struct drm_i915_gem_object *default_ctx_obj);
+/*
+ * Test to see if the ring has sufficient space to submit a given piece
+ * of work without causing a stall
+ */
+static int logical_ring_test_space(struct intel_ringbuffer *ringbuf,
+ int min_space)
+{
+ if (ringbuf->space < min_space) {
+ /* Need to update the actual ring space. Otherwise, the system
+ * hangs forever testing a software copy of the space value that
+ * never changes!
+ */
+ intel_ring_update_space(ringbuf);
+
+ if (ringbuf->space < min_space)
+ return -EAGAIN;
+ }
+
+ return 0;
+}
+
/**
* intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
* @dev: DRM device.
@@ -932,6 +953,7 @@ int intel_execlists_submission_final(struct i915_execbuffer_params *params)
struct intel_engine_cs *ring = params->ring;
u64 exec_start;
int ret;
+ uint32_t min_space;
/* The mutex must be acquired before calling this function */
WARN_ON(!mutex_is_locked(¶ms->dev->struct_mutex));
@@ -955,6 +977,34 @@ int intel_execlists_submission_final(struct i915_execbuffer_params *params)
return ret;
/*
+ * It would be a bad idea to run out of space while writing commands
+ * to the ring. One of the major aims of the scheduler is to not
+ * stall at any point for any reason. However, doing an early exit
+ * half way through submission could result in a partial sequence
+ * being written which would leave the engine in an unknown state.
+ * Therefore, check in advance that there will be enough space for
+ * the entire submission whether emitted by the code below OR by any
+ * other functions that may be executed before the end of final().
+ *
+ * NB: This test deliberately overestimates, because that's easier
+ * than tracing every potential path that could be taken!
+ *
+ * Current measurements suggest that we may need to emit up to 186
+ * dwords, so this is rounded up to 256 here. Then double that to get
+ * the free space requirement, because the block is not allowed to
+ * span the transition from the end to the beginning of the ring.
+ */
+#define I915_BATCH_EXEC_MAX_LEN 256 /* max dwords emitted here */
+ min_space = I915_BATCH_EXEC_MAX_LEN * 2 * sizeof(uint32_t);
+ ret = logical_ring_test_space(ringbuf, min_space);
+ if (ret)
+ goto err;
+
+ ret = intel_logical_ring_begin(req, I915_BATCH_EXEC_MAX_LEN);
+ if (ret)
+ goto err;
+
+ /*
* Unconditionally invalidate gpu caches and ensure that we do flush
* any residual writes from the previous batch.
*/
@@ -964,10 +1014,6 @@ int intel_execlists_submission_final(struct i915_execbuffer_params *params)
if (ring == &dev_priv->ring[RCS] &&
params->instp_mode != dev_priv->relative_constants_mode) {
- ret = intel_logical_ring_begin(req, 4);
- if (ret)
- return ret;
-
intel_logical_ring_emit(ringbuf, MI_NOOP);
intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
intel_logical_ring_emit(ringbuf, INSTPM);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3f3d928..477235c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2472,6 +2472,32 @@ int intel_ring_cacheline_align(struct drm_i915_gem_request *req)
return 0;
}
+/*
+ * Test to see if the ring has sufficient space to submit a given piece
+ * of work without causing a stall
+ */
+int intel_ring_test_space(struct intel_ringbuffer *ringbuf, int min_space)
+{
+ struct drm_i915_private *dev_priv = ringbuf->ring->dev->dev_private;
+
+ /* There is a separate LRC version of this code. */
+ WARN_ON(i915.enable_execlists);
+
+ if (ringbuf->space < min_space) {
+ /* Need to update the actual ring space. Otherwise, the system
+ * hangs forever testing a software copy of the space value that
+ * never changes!
+ */
+ ringbuf->head = I915_READ_HEAD(ringbuf->ring);
+ ringbuf->space = intel_ring_space(ringbuf);
+
+ if (ringbuf->space < min_space)
+ return -EAGAIN;
+ }
+
+ return 0;
+}
+
void intel_ring_init_seqno(struct intel_engine_cs *ring, u32 seqno)
{
struct drm_device *dev = ring->dev;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 136a039..bc447aa 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -446,6 +446,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring);
int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request);
+int intel_ring_test_space(struct intel_ringbuffer *ringbuf, int min_space);
int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
static inline void intel_ring_emit(struct intel_engine_cs *ring,
--
1.9.1
More information about the Intel-gfx
mailing list