From 6e1281412ab9e6772d8f6c26e592181fcdd2ae0c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 14 Nov 2017 22:33:46 +0000 Subject: drm/i915/selftests: Always initialise err smatch does not track initialised values as well as gcc, and this triggers many warnings by smatch not presented by gcc. Silence smatch by initialising the error values to -ENODEV, which we use to denote internal errors. (If we see a selftest fail with a silent -ENODEV, we know smatch was right!) v2: smatch was right about igt_create_vma(), it may unlikely fail on the first object allocation which we want to be loud about. Signed-off-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20171114223346.25958-1-chris@chris-wilson.co.uk Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/selftests/i915_gem_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/selftests/i915_gem_request.c') diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c b/drivers/gpu/drm/i915/selftests/i915_gem_request.c index a999161e8db1..6bce99050e94 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c @@ -332,7 +332,7 @@ static int live_nop_request(void *arg) struct intel_engine_cs *engine; struct live_test t; unsigned int id; - int err; + int err = -ENODEV; /* Submit various sized batches of empty requests, to each engine * (individually), and wait for the batch to complete. We can check -- cgit v1.2.3 From 2113184c6f6749f6e4e86a42894f67a50ead6775 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 20 Nov 2017 10:20:01 +0000 Subject: drm/i915: Pull the unconditional GPU cache invalidation into request construction As the request will, in the following patch, implicitly invoke a context-switch on construction, we should precede that with a GPU TLB invalidation. Also, even before using GGTT, we always want to invalidate the TLBs for any updates (as well as the ppgtt invalidates that are unconditionally applied by execbuf). Since we almost always require the TLB invalidate, do it unconditionally on request allocation and so we can remove it from all other paths. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20171120102002.22254-1-chris@chris-wilson.co.uk Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +------ drivers/gpu/drm/i915/i915_gem_render_state.c | 4 ---- drivers/gpu/drm/i915/i915_gem_request.c | 24 ++++++++++++++++++----- 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 ---- 7 files changed, 20 insertions(+), 37 deletions(-) (limited to 'drivers/gpu/drm/i915/selftests/i915_gem_request.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 53ccb27bfe91..b7895788bc75 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1111,10 +1111,6 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb, if (err) goto err_request; - err = eb->engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - goto err_request; - err = i915_switch_context(rq); if (err) goto err_request; @@ -1818,8 +1814,7 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb) /* Unconditionally flush any chipset caches (for streaming writes). */ i915_gem_chipset_flush(eb->i915); - /* Unconditionally invalidate GPU caches and TLBs. */ - return eb->engine->emit_flush(eb->request, EMIT_INVALIDATE); + return 0; } static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec) diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index c2723a06fbb4..f7fc0df251ac 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -208,10 +208,6 @@ int i915_gem_render_state_emit(struct drm_i915_gem_request *rq) if (err) goto err_unpin; - err = engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - goto err_unpin; - err = engine->emit_bb_start(rq, so.batch_offset, so.batch_size, I915_DISPATCH_SECURE); diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index e0d6221022a8..91eae1b20c42 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -703,17 +703,31 @@ 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 + /* + * 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 * position of the head. */ req->head = req->ring->emit; + /* Unconditionally invalidate GPU caches and TLBs. */ + ret = engine->emit_flush(req, EMIT_INVALIDATE); + if (ret) + goto err_ctx; + + ret = engine->request_alloc(req); + if (ret) { + /* + * Past the point-of-no-return. Since we may have updated + * global state after partially completing the request alloc, + * we need to commit any commands so far emitted in the + * request to the HW. + */ + __i915_add_request(req, false); + return ERR_PTR(ret); + } + /* Check that we didn't interrupt ourselves with a new request */ GEM_BUG_ON(req->timeline->seqno != req->fence.seqno); return req; diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index 01af540b6ef9..159a2cb68765 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -989,10 +989,6 @@ static int gpu_write(struct i915_vma *vma, i915_vma_unpin(batch); i915_vma_close(batch); - err = rq->engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - goto err_request; - err = i915_switch_context(rq); if (err) goto err_request; diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c index c82780a9d455..4ff30b9af1fe 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c @@ -158,10 +158,6 @@ static int gpu_fill(struct drm_i915_gem_object *obj, goto err_batch; } - err = engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - goto err_request; - err = i915_switch_context(rq); if (err) goto err_request; diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c b/drivers/gpu/drm/i915/selftests/i915_gem_request.c index 6bce99050e94..d7bf53ff8f84 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c @@ -459,10 +459,6 @@ empty_request(struct intel_engine_cs *engine, if (IS_ERR(request)) return request; - err = engine->emit_flush(request, EMIT_INVALIDATE); - if (err) - goto out_request; - err = i915_switch_context(request); if (err) goto out_request; @@ -675,9 +671,6 @@ static int live_all_engines(void *arg) goto out_request; } - err = engine->emit_flush(request[id], EMIT_INVALIDATE); - GEM_BUG_ON(err); - err = i915_switch_context(request[id]); GEM_BUG_ON(err); @@ -797,9 +790,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); diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c index 71ce06680d66..145bdc26553c 100644 --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -114,10 +114,6 @@ static int emit_recurse_batch(struct hang *h, if (err) goto unpin_vma; - err = rq->engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - goto unpin_hws; - err = i915_switch_context(rq); if (err) goto unpin_hws; -- cgit v1.2.3 From 3fef5cda970124a15c553c1672d800e40fc08a9e Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 20 Nov 2017 10:20:02 +0000 Subject: drm/i915: Automatic i915_switch_context for legacy 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 Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20171120102002.22254-2-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 | 4 ++++ drivers/gpu/drm/i915/i915_perf.c | 3 +-- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++++ drivers/gpu/drm/i915/selftests/huge_pages.c | 10 +++------- 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 | 5 ----- 10 files changed, 14 insertions(+), 43 deletions(-) (limited to 'drivers/gpu/drm/i915/selftests/i915_gem_request.c') diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 61ba321e9970..e07eb0beef13 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5045,7 +5045,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 b7895788bc75..14d9e61a1e06 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1111,10 +1111,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); @@ -1960,10 +1956,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 91eae1b20c42..86e2346357cf 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); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 0f48e666098d..fd150099978c 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 159a2cb68765..db7a0a1f2960 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -989,13 +989,9 @@ static int gpu_write(struct i915_vma *vma, i915_vma_unpin(batch); i915_vma_close(batch); - err = i915_switch_context(rq); - if (err) - goto err_request; - - err = rq->engine->emit_bb_start(rq, - batch->node.start, batch->node.size, - flags); + err = engine->emit_bb_start(rq, + batch->node.start, batch->node.size, + flags); if (err) goto err_request; diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c index 4ff30b9af1fe..09340b3c1156 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c @@ -158,10 +158,6 @@ static int gpu_fill(struct drm_i915_gem_object *obj, goto err_batch; } - 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 d7bf53ff8f84..647bf2bbd799 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c @@ -459,10 +459,6 @@ empty_request(struct intel_engine_cs *engine, if (IS_ERR(request)) return request; - err = i915_switch_context(request); - if (err) - goto out_request; - err = engine->emit_bb_start(request, batch->node.start, batch->node.size, @@ -671,9 +667,6 @@ static int live_all_engines(void *arg) goto out_request; } - err = i915_switch_context(request[id]); - GEM_BUG_ON(err); - err = engine->emit_bb_start(request[id], batch->node.start, batch->node.size, @@ -790,9 +783,6 @@ static int live_sequential_engines(void *arg) } } - 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 145bdc26553c..1bbb8c46e2d9 100644 --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -114,10 +114,6 @@ static int emit_recurse_batch(struct hang *h, if (err) goto unpin_vma; - 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); @@ -169,7 +165,6 @@ static int emit_recurse_batch(struct hang *h, err = rq->engine->emit_bb_start(rq, vma->node.start, PAGE_SIZE, flags); -unpin_hws: i915_vma_unpin(hws); unpin_vma: i915_vma_unpin(vma); -- cgit v1.2.3