* [PATCH 2/3] drm/i915: Make request's wait-for-space explicit
2017-11-14 11:41 [PATCH 1/3] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
@ 2017-11-14 11:41 ` Chris Wilson
2017-11-14 11:41 ` [PATCH 3/3] drm/i915: Automatic i915_switch_context for legacy Chris Wilson
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-11-14 11:41 UTC (permalink / raw)
To: intel-gfx
At the start of building a request, we would wait for roughly enough
space to fit the average request (to reduce the likelihood of having to
wait and abort partway through request construction). To achieve we
would try to begin a 0-length command packet, this just adds extra
confusion so make the wait-for-space explicit, as in the next patch we
want to move it from the backend to the i915_gem_request_alloc() so it
can ensure that the wait-for-space is the first operation in building a
new request.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_lrc.c | 8 ++---
drivers/gpu/drm/i915/intel_ringbuffer.c | 56 +++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
3 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 58d050a9a866..ebd9596fe83b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1180,7 +1180,7 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
{
struct intel_engine_cs *engine = request->engine;
struct intel_context *ce = &request->ctx->engine[engine->id];
- u32 *cs;
+ int ret;
GEM_BUG_ON(!ce->pin_count);
@@ -1190,9 +1190,9 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
*/
request->reserved_space += EXECLISTS_REQUEST_SIZE;
- cs = intel_ring_begin(request, 0);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
+ ret = intel_ring_wait_for_space(request->ring, request->reserved_space);
+ if (ret)
+ return ret;
/* Note that after this point, we have committed to using
* this request as it is being used to both track the
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3321b801e77d..12e734b29463 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1578,7 +1578,7 @@ void intel_legacy_submission_resume(struct drm_i915_private *dev_priv)
static int ring_request_alloc(struct drm_i915_gem_request *request)
{
- u32 *cs;
+ int ret;
GEM_BUG_ON(!request->ctx->engine[request->engine->id].pin_count);
@@ -1588,37 +1588,24 @@ static int ring_request_alloc(struct drm_i915_gem_request *request)
*/
request->reserved_space += LEGACY_REQUEST_SIZE;
- cs = intel_ring_begin(request, 0);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
+ ret = intel_ring_wait_for_space(request->ring, request->reserved_space);
+ if (ret)
+ return ret;
request->reserved_space -= LEGACY_REQUEST_SIZE;
return 0;
}
-static noinline int wait_for_space(struct drm_i915_gem_request *req,
- unsigned int bytes)
+static noinline int wait_for_space(struct intel_ring *ring, unsigned int bytes)
{
- struct intel_ring *ring = req->ring;
struct drm_i915_gem_request *target;
long timeout;
- lockdep_assert_held(&req->i915->drm.struct_mutex);
+ lockdep_assert_held(&ring->vma->vm->i915->drm.struct_mutex);
if (intel_ring_update_space(ring) >= bytes)
return 0;
- /*
- * Space is reserved in the ringbuffer for finalising the request,
- * as that cannot be allowed to fail. During request finalisation,
- * reserved_space is set to 0 to stop the overallocation and the
- * assumption is that then we never need to wait (which has the
- * risk of failing with EINTR).
- *
- * See also i915_gem_request_alloc() and i915_add_request().
- */
- GEM_BUG_ON(!req->reserved_space);
-
list_for_each_entry(target, &ring->request_list, ring_link) {
/* Would completion of this request free enough space? */
if (bytes <= __intel_ring_space(target->postfix,
@@ -1642,6 +1629,22 @@ static noinline int wait_for_space(struct drm_i915_gem_request *req,
return 0;
}
+int intel_ring_wait_for_space(struct intel_ring *ring, unsigned int bytes)
+{
+ GEM_BUG_ON(bytes > ring->effective_size);
+ if (unlikely(bytes > ring->effective_size - ring->emit))
+ bytes += ring->size - ring->emit;
+
+ if (unlikely(bytes > ring->space)) {
+ int ret = wait_for_space(ring, bytes);
+ if (unlikely(ret))
+ return ret;
+ }
+
+ GEM_BUG_ON(ring->space < bytes);
+ return 0;
+}
+
u32 *intel_ring_begin(struct drm_i915_gem_request *req,
unsigned int num_dwords)
{
@@ -1681,7 +1684,20 @@ u32 *intel_ring_begin(struct drm_i915_gem_request *req,
}
if (unlikely(total_bytes > ring->space)) {
- int ret = wait_for_space(req, total_bytes);
+ int ret;
+
+ /*
+ * Space is reserved in the ringbuffer for finalising the
+ * request, as that cannot be allowed to fail. During request
+ * finalisation, reserved_space is set to 0 to stop the
+ * overallocation and the assumption is that then we never need
+ * to wait (which has the risk of failing with EINTR).
+ *
+ * See also i915_gem_request_alloc() and i915_add_request().
+ */
+ GEM_BUG_ON(!req->reserved_space);
+
+ ret = wait_for_space(ring, total_bytes);
if (unlikely(ret))
return ERR_PTR(ret);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2b0ae5468f48..0531d939fe1a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -661,6 +661,7 @@ void intel_legacy_submission_resume(struct drm_i915_private *dev_priv);
int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
+int intel_ring_wait_for_space(struct intel_ring *ring, unsigned int bytes);
u32 __must_check *intel_ring_begin(struct drm_i915_gem_request *req,
unsigned int n);
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] drm/i915: Automatic i915_switch_context for legacy
2017-11-14 11:41 [PATCH 1/3] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
2017-11-14 11:41 ` [PATCH 2/3] drm/i915: Make request's wait-for-space explicit Chris Wilson
@ 2017-11-14 11:41 ` Chris Wilson
2017-11-14 12:23 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/selftests: Increase size for mock ringbuffer Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-11-14 11:41 UTC (permalink / raw)
To: intel-gfx
During request construction, after pinning the context we know whether
or not we have to emit a context switch. So move this common operation
from every caller into i915_gem_request_alloc() itself.
v2: Always submit the request if we emitted some commands during request
construction, as typically it also involves changes in global state.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 7 +------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 --------
drivers/gpu/drm/i915/i915_gem_request.c | 18 +++++++++++++-----
drivers/gpu/drm/i915/i915_perf.c | 3 +--
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++++
drivers/gpu/drm/i915/selftests/huge_pages.c | 4 ----
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 4 ----
drivers/gpu/drm/i915/selftests/i915_gem_request.c | 10 ----------
drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 4 ----
10 files changed, 20 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fa9e5130c986..40fad8c1ebad 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5016,7 +5016,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
goto out_ctx;
}
- err = i915_switch_context(rq);
+ err = 0;
if (engine->init_context)
err = engine->init_context(rq);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2db040695035..c1efbaf02bf2 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -842,8 +842,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
struct intel_engine_cs *engine = req->engine;
lockdep_assert_held(&req->i915->drm.struct_mutex);
- if (i915_modparams.enable_execlists)
- return 0;
+ GEM_BUG_ON(i915_modparams.enable_execlists);
if (!req->ctx->engine[engine->id].state) {
struct i915_gem_context *to = req->ctx;
@@ -899,7 +898,6 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
for_each_engine(engine, dev_priv, id) {
struct drm_i915_gem_request *req;
- int ret;
if (engine_has_idle_kernel_context(engine))
continue;
@@ -922,10 +920,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
GFP_KERNEL);
}
- ret = i915_switch_context(req);
i915_add_request(req);
- if (ret)
- return ret;
}
return 0;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 435ed95df144..85c7e8afe26e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1115,10 +1115,6 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
if (err)
goto err_request;
- err = i915_switch_context(rq);
- if (err)
- goto err_request;
-
err = eb->engine->emit_bb_start(rq,
batch->node.start, PAGE_SIZE,
cache->gen > 5 ? 0 : I915_DISPATCH_SECURE);
@@ -1965,10 +1961,6 @@ static int eb_submit(struct i915_execbuffer *eb)
if (err)
return err;
- err = i915_switch_context(eb->request);
- if (err)
- return err;
-
if (eb->args->flags & I915_EXEC_GEN7_SOL_RESET) {
err = i915_reset_gen7_sol_offsets(eb->request);
if (err)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index e0d6221022a8..445495f9893c 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -624,6 +624,10 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
if (ret)
goto err_unpin;
+ ret = intel_ring_wait_for_space(ring, MIN_SPACE_FOR_ADD_REQUEST);
+ if (ret)
+ goto err_unreserve;
+
/* Move the oldest request to the slab-cache (if not in use!) */
req = list_first_entry_or_null(&engine->timeline->requests,
typeof(*req), link);
@@ -703,10 +707,6 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST;
GEM_BUG_ON(req->reserved_space < engine->emit_breadcrumb_sz);
- ret = engine->request_alloc(req);
- if (ret)
- goto err_ctx;
-
/* Record the position of the start of the request so that
* should we detect the updated seqno part-way through the
* GPU processing the request, we never over-estimate the
@@ -714,16 +714,24 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
*/
req->head = req->ring->emit;
+ ret = engine->request_alloc(req);
+ if (ret)
+ goto err_ctx;
+
/* Check that we didn't interrupt ourselves with a new request */
GEM_BUG_ON(req->timeline->seqno != req->fence.seqno);
return req;
err_ctx:
+ if (req->ring->emit != req->head) {
+ __i915_add_request(req, false);
+ return ERR_PTR(ret);
+ }
+
/* Make sure we didn't add ourselves to external state before freeing */
GEM_BUG_ON(!list_empty(&req->active_list));
GEM_BUG_ON(!list_empty(&req->priotree.signalers_list));
GEM_BUG_ON(!list_empty(&req->priotree.waiters_list));
-
kmem_cache_free(dev_priv->requests, req);
err_unreserve:
unreserve_engine(engine);
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 00be015e01df..d8952ff8e6b7 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1726,10 +1726,9 @@ static int gen8_switch_to_updated_kernel_context(struct drm_i915_private *dev_pr
GFP_KERNEL);
}
- ret = i915_switch_context(req);
i915_add_request(req);
- return ret;
+ return 0;
}
/*
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 12e734b29463..be98868115bf 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1592,6 +1592,10 @@ static int ring_request_alloc(struct drm_i915_gem_request *request)
if (ret)
return ret;
+ ret = i915_switch_context(request);
+ if (ret)
+ return ret;
+
request->reserved_space -= LEGACY_REQUEST_SIZE;
return 0;
}
diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
index 01af540b6ef9..eb142bd4c9e1 100644
--- a/drivers/gpu/drm/i915/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
@@ -993,10 +993,6 @@ static int gpu_write(struct i915_vma *vma,
if (err)
goto err_request;
- err = i915_switch_context(rq);
- if (err)
- goto err_request;
-
err = rq->engine->emit_bb_start(rq,
batch->node.start, batch->node.size,
flags);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index def5052862ae..61fcfa2c4dfd 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -162,10 +162,6 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
if (err)
goto err_request;
- err = i915_switch_context(rq);
- if (err)
- goto err_request;
-
flags = 0;
if (INTEL_GEN(vm->i915) <= 5)
flags |= I915_DISPATCH_SECURE;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
index a999161e8db1..9a35ebd5c876 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
@@ -463,10 +463,6 @@ empty_request(struct intel_engine_cs *engine,
if (err)
goto out_request;
- err = i915_switch_context(request);
- if (err)
- goto out_request;
-
err = engine->emit_bb_start(request,
batch->node.start,
batch->node.size,
@@ -678,9 +674,6 @@ static int live_all_engines(void *arg)
err = engine->emit_flush(request[id], EMIT_INVALIDATE);
GEM_BUG_ON(err);
- err = i915_switch_context(request[id]);
- GEM_BUG_ON(err);
-
err = engine->emit_bb_start(request[id],
batch->node.start,
batch->node.size,
@@ -800,9 +793,6 @@ static int live_sequential_engines(void *arg)
err = engine->emit_flush(request[id], EMIT_INVALIDATE);
GEM_BUG_ON(err);
- err = i915_switch_context(request[id]);
- GEM_BUG_ON(err);
-
err = engine->emit_bb_start(request[id],
batch->node.start,
batch->node.size,
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index 71ce06680d66..cafe39e2e0f7 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -118,10 +118,6 @@ static int emit_recurse_batch(struct hang *h,
if (err)
goto unpin_hws;
- err = i915_switch_context(rq);
- if (err)
- goto unpin_hws;
-
i915_vma_move_to_active(vma, rq, 0);
if (!i915_gem_object_has_active_reference(vma->obj)) {
i915_gem_object_get(vma->obj);
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread