* [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer
@ 2017-11-14 21:30 Chris Wilson
2017-11-14 21:30 ` [PATCH 2/8] drm/i915: Make request's wait-for-space explicit Chris Wilson
` (9 more replies)
0 siblings, 10 replies; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 UTC (permalink / raw)
To: intel-gfx
We don't actually emit any commands into the ringbuffer, so we set it
very small. However, an upcoming change centralises the wait-for-space
into i915_gem_request_alloc() and that imposes a minimum size upon all
ringbuffers (mock or real) of MIN_SPACE_FOR_ADD_REQUEST. Grow the
mock ringbuffer such that we allocate a single page for the struct+buffer,
satisfying the new condition without wasting too much space.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/selftests/mock_engine.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index 0aafa8a105b8..55c0e2c15782 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -124,9 +124,11 @@ static void mock_submit_request(struct drm_i915_gem_request *request)
static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
{
- const unsigned long sz = roundup_pow_of_two(sizeof(struct intel_ring));
+ const unsigned long sz = PAGE_SIZE / 2;
struct intel_ring *ring;
+ BUILD_BUG_ON(MIN_SPACE_FOR_ADD_REQUEST > sz);
+
ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
if (!ring)
return NULL;
--
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] 20+ messages in thread
* [PATCH 2/8] drm/i915: Make request's wait-for-space explicit
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
[not found] ` <87shdfps4u.fsf@gaia.fi.intel.com>
2017-11-14 21:30 ` [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy Chris Wilson
` (8 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 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] 20+ messages in thread
* [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
2017-11-14 21:30 ` [PATCH 2/8] drm/i915: Make request's wait-for-space explicit Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
2017-11-16 14:00 ` Mika Kuoppala
2017-11-14 21:30 ` [PATCH 4/8] drm/i915: Remove i915.enable_execlists module parameter Chris Wilson
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 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 a7979b74ce21..d885cf0d2943 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5043,7 +5043,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] 20+ messages in thread
* [PATCH 4/8] drm/i915: Remove i915.enable_execlists module parameter
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
2017-11-14 21:30 ` [PATCH 2/8] drm/i915: Make request's wait-for-space explicit Chris Wilson
2017-11-14 21:30 ` [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
2017-11-14 21:30 ` [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+ Chris Wilson
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 UTC (permalink / raw)
To: intel-gfx
Execlists and legacy ringbuffer submission are no longer feature
comparable (execlists now offer greater functionality that should
overcome their performance hit) and obsoletes the unsafe module
parameter, i.e. comparing the two modes of execution is no longer
useful, so remove the debug tool.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> #i915_perf.c
---
drivers/gpu/drm/i915/gvt/render.c | 3 +-
drivers/gpu/drm/i915/i915_debugfs.c | 70 ---------------------------------
drivers/gpu/drm/i915/i915_drv.c | 8 +---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_gem.c | 10 ++---
drivers/gpu/drm/i915/i915_gem_context.c | 10 +----
drivers/gpu/drm/i915/i915_gem_gtt.c | 4 +-
drivers/gpu/drm/i915/i915_params.c | 4 --
drivers/gpu/drm/i915/i915_params.h | 1 -
drivers/gpu/drm/i915/i915_perf.c | 8 ++--
drivers/gpu/drm/i915/intel_engine_cs.c | 8 ++--
drivers/gpu/drm/i915/intel_gvt.c | 5 ---
drivers/gpu/drm/i915/intel_lrc.c | 31 ---------------
drivers/gpu/drm/i915/intel_lrc.h | 4 --
14 files changed, 20 insertions(+), 149 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index 6d066cf35478..99d66b4f2965 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -292,8 +292,7 @@ static void switch_mmio_to_vgpu(struct intel_vgpu *vgpu, int ring_id)
* write.
*/
if (mmio->in_context &&
- ((ctx_ctrl & inhibit_mask) != inhibit_mask) &&
- i915_modparams.enable_execlists)
+ (ctx_ctrl & inhibit_mask) != inhibit_mask)
continue;
if (mmio->mask)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6ba08b0c1c22..f705e4d7d5a5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1989,75 +1989,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
return 0;
}
-static void i915_dump_lrc_obj(struct seq_file *m,
- struct i915_gem_context *ctx,
- struct intel_engine_cs *engine)
-{
- struct i915_vma *vma = ctx->engine[engine->id].state;
- struct page *page;
- int j;
-
- seq_printf(m, "CONTEXT: %s %u\n", engine->name, ctx->hw_id);
-
- if (!vma) {
- seq_puts(m, "\tFake context\n");
- return;
- }
-
- if (vma->flags & I915_VMA_GLOBAL_BIND)
- seq_printf(m, "\tBound in GGTT at 0x%08x\n",
- i915_ggtt_offset(vma));
-
- if (i915_gem_object_pin_pages(vma->obj)) {
- seq_puts(m, "\tFailed to get pages for context object\n\n");
- return;
- }
-
- page = i915_gem_object_get_page(vma->obj, LRC_STATE_PN);
- if (page) {
- u32 *reg_state = kmap_atomic(page);
-
- for (j = 0; j < 0x600 / sizeof(u32) / 4; j += 4) {
- seq_printf(m,
- "\t[0x%04x] 0x%08x 0x%08x 0x%08x 0x%08x\n",
- j * 4,
- reg_state[j], reg_state[j + 1],
- reg_state[j + 2], reg_state[j + 3]);
- }
- kunmap_atomic(reg_state);
- }
-
- i915_gem_object_unpin_pages(vma->obj);
- seq_putc(m, '\n');
-}
-
-static int i915_dump_lrc(struct seq_file *m, void *unused)
-{
- struct drm_i915_private *dev_priv = node_to_i915(m->private);
- struct drm_device *dev = &dev_priv->drm;
- struct intel_engine_cs *engine;
- struct i915_gem_context *ctx;
- enum intel_engine_id id;
- int ret;
-
- if (!i915_modparams.enable_execlists) {
- seq_printf(m, "Logical Ring Contexts are disabled\n");
- return 0;
- }
-
- ret = mutex_lock_interruptible(&dev->struct_mutex);
- if (ret)
- return ret;
-
- list_for_each_entry(ctx, &dev_priv->contexts.list, link)
- for_each_engine(engine, dev_priv, id)
- i915_dump_lrc_obj(m, ctx, engine);
-
- mutex_unlock(&dev->struct_mutex);
-
- return 0;
-}
-
static const char *swizzle_string(unsigned swizzle)
{
switch (swizzle) {
@@ -4820,7 +4751,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_vbt", i915_vbt, 0},
{"i915_gem_framebuffer", i915_gem_framebuffer_info, 0},
{"i915_context_status", i915_context_status, 0},
- {"i915_dump_lrc", i915_dump_lrc, 0},
{"i915_forcewake_domains", i915_forcewake_domains, 0},
{"i915_swizzle_info", i915_swizzle_info, 0},
{"i915_ppgtt_info", i915_ppgtt_info, 0},
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 171b86f37878..abf1073bee88 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -371,9 +371,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule) {
value |= I915_SCHEDULER_CAP_ENABLED;
value |= I915_SCHEDULER_CAP_PRIORITY;
-
- if (HAS_LOGICAL_RING_PREEMPTION(dev_priv) &&
- i915_modparams.enable_execlists)
+ if (HAS_LOGICAL_RING_PREEMPTION(dev_priv))
value |= I915_SCHEDULER_CAP_PREEMPTION;
}
break;
@@ -1052,10 +1050,6 @@ static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
static void intel_sanitize_options(struct drm_i915_private *dev_priv)
{
- i915_modparams.enable_execlists =
- intel_sanitize_enable_execlists(dev_priv,
- i915_modparams.enable_execlists);
-
/*
* i915.enable_ppgtt is read-only, so do an early pass to validate the
* user's requested state against the hardware/driver capabilities. We
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2158a758a17d..909cd53fa8c4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3154,6 +3154,9 @@ intel_info(const struct drm_i915_private *dev_priv)
((dev_priv)->info.has_logical_ring_contexts)
#define HAS_LOGICAL_RING_PREEMPTION(dev_priv) \
((dev_priv)->info.has_logical_ring_preemption)
+
+#define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv)
+
#define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt)
#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2)
#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d885cf0d2943..af122bf354e5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5001,7 +5001,7 @@ bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
return false;
/* TODO: make semaphores and Execlists play nicely together */
- if (i915_modparams.enable_execlists)
+ if (HAS_EXECLISTS(dev_priv))
return false;
if (value >= 0)
@@ -5145,12 +5145,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
dev_priv->mm.unordered_timeline = dma_fence_context_alloc(1);
- if (!i915_modparams.enable_execlists) {
- dev_priv->gt.resume = intel_legacy_submission_resume;
- dev_priv->gt.cleanup_engine = intel_engine_cleanup;
- } else {
+ if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
dev_priv->gt.resume = intel_lr_context_resume;
dev_priv->gt.cleanup_engine = intel_logical_ring_cleanup;
+ } else {
+ dev_priv->gt.resume = intel_legacy_submission_resume;
+ dev_priv->gt.cleanup_engine = intel_engine_cleanup;
}
/* This is just a security blanket to placate dragons.
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index c1efbaf02bf2..c75941d3d7e7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -460,14 +460,6 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
INIT_WORK(&dev_priv->contexts.free_work, contexts_free_worker);
init_llist_head(&dev_priv->contexts.free_list);
- if (intel_vgpu_active(dev_priv) &&
- HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
- if (!i915_modparams.enable_execlists) {
- DRM_INFO("Only EXECLIST mode is supported in vgpu.\n");
- return -EINVAL;
- }
- }
-
/* Using the simple ida interface, the max is limited by sizeof(int) */
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
ida_init(&dev_priv->contexts.hw_ida);
@@ -842,7 +834,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);
- GEM_BUG_ON(i915_modparams.enable_execlists);
+ GEM_BUG_ON(HAS_EXECLISTS(req->i915));
if (!req->ctx->engine[engine->id].state) {
struct i915_gem_context *to = req->ctx;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 3c3a699436c9..922cf5fd3dd7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -178,7 +178,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
return 0;
}
- if (INTEL_GEN(dev_priv) >= 8 && i915_modparams.enable_execlists) {
+ if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
if (has_full_48bit_ppgtt)
return 3;
@@ -2162,7 +2162,7 @@ int i915_ppgtt_init_hw(struct drm_i915_private *dev_priv)
/* In the case of execlists, PPGTT is enabled by the context descriptor
* and the PDPs are contained within the context itself. We don't
* need to do anything here. */
- if (i915_modparams.enable_execlists)
+ if (HAS_LOGICAL_RING_CONTEXTS(dev_priv))
return 0;
if (!USES_PPGTT(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b4faeb6aa2bd..d61c1787c164 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -99,10 +99,6 @@ i915_param_named_unsafe(enable_ppgtt, int, 0400,
"Override PPGTT usage. "
"(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)");
-i915_param_named_unsafe(enable_execlists, int, 0400,
- "Override execlists usage. "
- "(-1=auto [default], 0=disabled, 1=enabled)");
-
i915_param_named_unsafe(enable_psr, int, 0600,
"Enable PSR "
"(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c7292268ed43..0aef3d7178eb 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -39,7 +39,6 @@
param(int, enable_dc, -1) \
param(int, enable_fbc, -1) \
param(int, enable_ppgtt, -1) \
- param(int, enable_execlists, -1) \
param(int, enable_psr, -1) \
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index d8952ff8e6b7..e8b2f9e7e4c3 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1216,9 +1216,9 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
{
struct drm_i915_private *dev_priv = stream->dev_priv;
- if (i915_modparams.enable_execlists)
+ if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
dev_priv->perf.oa.specific_ctx_id = stream->ctx->hw_id;
- else {
+ } else {
struct intel_engine_cs *engine = dev_priv->engine[RCS];
struct intel_ring *ring;
int ret;
@@ -1262,7 +1262,7 @@ static void oa_put_render_ctx_id(struct i915_perf_stream *stream)
{
struct drm_i915_private *dev_priv = stream->dev_priv;
- if (i915_modparams.enable_execlists) {
+ if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID;
} else {
struct intel_engine_cs *engine = dev_priv->engine[RCS];
@@ -3434,7 +3434,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
dev_priv->perf.oa.timestamp_frequency = 12500000;
dev_priv->perf.oa.oa_formats = hsw_oa_formats;
- } else if (i915_modparams.enable_execlists) {
+ } else if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
/* Note: that although we could theoretically also support the
* legacy ringbuffer mode on BDW (and earlier iterations of
* this driver, before upstreaming did this) it didn't seem
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a42b738e79e7..4258f94a1960 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -164,9 +164,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
case 9:
return GEN9_LR_CONTEXT_RENDER_SIZE;
case 8:
- return i915_modparams.enable_execlists ?
- GEN8_LR_CONTEXT_RENDER_SIZE :
- GEN8_CXT_TOTAL_SIZE;
+ return GEN8_LR_CONTEXT_RENDER_SIZE;
case 7:
if (IS_HASWELL(dev_priv))
return HSW_CXT_TOTAL_SIZE;
@@ -316,7 +314,7 @@ int intel_engines_init(struct drm_i915_private *dev_priv)
&intel_engine_classes[engine->class];
int (*init)(struct intel_engine_cs *engine);
- if (i915_modparams.enable_execlists)
+ if (HAS_EXECLISTS(dev_priv))
init = class_info->init_execlists;
else
init = class_info->init_legacy;
@@ -1739,7 +1737,7 @@ void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m)
drm_printf(m, "\tBBADDR: 0x%08x_%08x\n",
upper_32_bits(addr), lower_32_bits(addr));
- if (i915_modparams.enable_execlists) {
+ if (HAS_EXECLISTS(dev_priv)) {
const u32 *hws = &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
u32 ptr, read, write;
unsigned int idx;
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index b4a7f31f0214..126f7c769c69 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -95,11 +95,6 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
return 0;
}
- if (!i915_modparams.enable_execlists) {
- DRM_ERROR("i915 GVT-g loading failed due to disabled execlists mode\n");
- return -EIO;
- }
-
if (i915_modparams.enable_guc_submission) {
DRM_ERROR("i915 GVT-g loading failed due to Graphics virtualization is not yet supported with GuC submission\n");
return -EIO;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ebd9596fe83b..5a22bdffd387 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -220,37 +220,6 @@ static void execlists_init_reg_state(u32 *reg_state,
struct intel_engine_cs *engine,
struct intel_ring *ring);
-/**
- * intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
- * @dev_priv: i915 device private
- * @enable_execlists: value of i915.enable_execlists module parameter.
- *
- * Only certain platforms support Execlists (the prerequisites being
- * support for Logical Ring Contexts and Aliasing PPGTT or better).
- *
- * Return: 1 if Execlists is supported and has to be enabled.
- */
-int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv, int enable_execlists)
-{
- /* On platforms with execlist available, vGPU will only
- * support execlist mode, no ring buffer mode.
- */
- if (HAS_LOGICAL_RING_CONTEXTS(dev_priv) && intel_vgpu_active(dev_priv))
- return 1;
-
- if (INTEL_GEN(dev_priv) >= 9)
- return 1;
-
- if (enable_execlists == 0)
- return 0;
-
- if (HAS_LOGICAL_RING_CONTEXTS(dev_priv) &&
- USES_PPGTT(dev_priv))
- return 1;
-
- return 0;
-}
-
/**
* intel_lr_context_descriptor_update() - calculate & cache the descriptor
* descriptor for a pinned context
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 17182ce29674..6d4f9b995a11 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -107,8 +107,4 @@ intel_lr_context_descriptor(struct i915_gem_context *ctx,
return ctx->engine[engine->id].lrc_desc;
}
-/* Execlists */
-int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv,
- int enable_execlists);
-
#endif /* _INTEL_LRC_H_ */
--
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] 20+ messages in thread
* [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (2 preceding siblings ...)
2017-11-14 21:30 ` [PATCH 4/8] drm/i915: Remove i915.enable_execlists module parameter Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
2017-11-17 13:29 ` Mika Kuoppala
2017-11-14 21:30 ` [PATCH 6/8] drm/i915: Disable semaphores on Sandybridge Chris Wilson
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 UTC (permalink / raw)
To: intel-gfx
Since removing the module parameter to force selection of ringbuffer
emission for gen8, the code is defunct. Remove it.
To put the difference into perspective, a couple of microbenchmarks
(bdw i7-5557u, 20170324):
ring execlists
exec continuous nops on all rings: 1.491us 2.223us
exec sequential nops on each ring: 12.508us 53.682us
single nop + sync: 9.272us 30.291us
vblank_mode=0 glxgears: ~11000fps ~9000fps
Since the earlier submission, gen8 ringbuffer submission has fallen
further and further behind in features. So while ringbuffer may hold the
throughput crown, in terms of interactive latency, execlists is much
better. Alas, we have no convenient metrics for such, other than
demonstrating things we can do with execlists but can not using
legacy ringbuffer submission.
We have made a few improvements to lowlevel execlists throughput,
and ringbuffer currently panics on boot! (bdw i7-5557u, 20171026):
ring execlists
exec continuous nops on all rings: n/a 1.921us
exec sequential nops on each ring: n/a 44.621us
single nop + sync: n/a 21.953us
vblank_mode=0 glxgears: n/a ~18500fps
References: https://bugs.freedesktop.org/show_bug.cgi?id=87725
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Once-upon-a-time-Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 44 +---
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 47 +---
drivers/gpu/drm/i915/i915_gpu_error.c | 36 ---
drivers/gpu/drm/i915/intel_engine_cs.c | 12 -
drivers/gpu/drm/i915/intel_hangcheck.c | 44 +---
drivers/gpu/drm/i915/intel_ringbuffer.c | 431 +++++---------------------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 25 +-
9 files changed, 94 insertions(+), 549 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f705e4d7d5a5..c2375aaecb9a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3241,44 +3241,12 @@ static int i915_semaphore_status(struct seq_file *m, void *unused)
return ret;
intel_runtime_pm_get(dev_priv);
- if (IS_BROADWELL(dev_priv)) {
- struct page *page;
- uint64_t *seqno;
-
- page = i915_gem_object_get_page(dev_priv->semaphore->obj, 0);
-
- seqno = (uint64_t *)kmap_atomic(page);
- for_each_engine(engine, dev_priv, id) {
- uint64_t offset;
-
- seq_printf(m, "%s\n", engine->name);
-
- seq_puts(m, " Last signal:");
- for (j = 0; j < num_rings; j++) {
- offset = id * I915_NUM_ENGINES + j;
- seq_printf(m, "0x%08llx (0x%02llx) ",
- seqno[offset], offset * 8);
- }
- seq_putc(m, '\n');
-
- seq_puts(m, " Last wait: ");
- for (j = 0; j < num_rings; j++) {
- offset = id + (j * I915_NUM_ENGINES);
- seq_printf(m, "0x%08llx (0x%02llx) ",
- seqno[offset], offset * 8);
- }
- seq_putc(m, '\n');
-
- }
- kunmap_atomic(seqno);
- } else {
- seq_puts(m, " Last signal:");
- for_each_engine(engine, dev_priv, id)
- for (j = 0; j < num_rings; j++)
- seq_printf(m, "0x%08x\n",
- I915_READ(engine->semaphore.mbox.signal[j]));
- seq_putc(m, '\n');
- }
+ seq_puts(m, " Last signal:");
+ for_each_engine(engine, dev_priv, id)
+ for (j = 0; j < num_rings; j++)
+ seq_printf(m, "0x%08x\n",
+ I915_READ(engine->semaphore.mbox.signal[j]));
+ seq_putc(m, '\n');
intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 909cd53fa8c4..4172b67a3db0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -942,7 +942,6 @@ struct i915_gpu_state {
u64 fence[I915_MAX_NUM_FENCES];
struct intel_overlay_error_state *overlay;
struct intel_display_error_state *display;
- struct drm_i915_error_object *semaphore;
struct drm_i915_error_engine {
int engine_id;
@@ -2291,7 +2290,6 @@ struct drm_i915_private {
struct i915_gem_context *kernel_context;
/* Context only to be used for injecting preemption commands */
struct i915_gem_context *preempt_context;
- struct i915_vma *semaphore;
struct drm_dma_handle *status_page_dmah;
struct resource mch_res;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index af122bf354e5..93bfcaccad6c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4997,7 +4997,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
{
- if (INTEL_INFO(dev_priv)->gen < 6)
+ if (INTEL_GEN(dev_priv) < 6)
return false;
/* TODO: make semaphores and Execlists play nicely together */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index c75941d3d7e7..0704d9af261b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -574,21 +574,21 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
enum intel_engine_id id;
const int num_rings =
/* Use an extended w/a on gen7 if signalling from other rings */
- (i915_modparams.semaphores && INTEL_GEN(dev_priv) == 7) ?
+ (i915_modparams.semaphores && IS_GEN7(dev_priv)) ?
INTEL_INFO(dev_priv)->num_rings - 1 :
0;
int len;
u32 *cs;
flags |= MI_MM_SPACE_GTT;
- if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
+ if (IS_HASWELL(dev_priv))
/* These flags are for resource streamer on HSW+ */
flags |= HSW_MI_RS_SAVE_STATE_EN | HSW_MI_RS_RESTORE_STATE_EN;
else
flags |= MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN;
len = 4;
- if (INTEL_GEN(dev_priv) >= 7)
+ if (IS_GEN7(dev_priv))
len += 2 + (num_rings ? 4*num_rings + 6 : 0);
cs = intel_ring_begin(req, len);
@@ -596,7 +596,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
return PTR_ERR(cs);
/* WaProgramMiArbOnOffAroundMiSetContext:ivb,vlv,hsw,bdw,chv */
- if (INTEL_GEN(dev_priv) >= 7) {
+ if (IS_GEN7(dev_priv)) {
*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
if (num_rings) {
struct intel_engine_cs *signaller;
@@ -623,7 +623,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
*/
*cs++ = MI_NOOP;
- if (INTEL_GEN(dev_priv) >= 7) {
+ if (IS_GEN7(dev_priv)) {
if (num_rings) {
struct intel_engine_cs *signaller;
i915_reg_t last_reg = {}; /* keep gcc quiet */
@@ -714,27 +714,7 @@ needs_pd_load_pre(struct i915_hw_ppgtt *ppgtt, struct intel_engine_cs *engine)
if (engine->id != RCS)
return true;
- if (INTEL_GEN(engine->i915) < 8)
- return true;
-
- return false;
-}
-
-static bool
-needs_pd_load_post(struct i915_hw_ppgtt *ppgtt,
- struct i915_gem_context *to,
- u32 hw_flags)
-{
- if (!ppgtt)
- return false;
-
- if (!IS_GEN8(to->i915))
- return false;
-
- if (hw_flags & MI_RESTORE_INHIBIT)
- return true;
-
- return false;
+ return true;
}
static int do_rcs_switch(struct drm_i915_gem_request *req)
@@ -784,21 +764,6 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
engine->legacy_active_context = to;
}
- /* GEN8 does *not* require an explicit reload if the PDPs have been
- * setup, and we do not wish to move them.
- */
- if (needs_pd_load_post(ppgtt, to, hw_flags)) {
- trace_switch_mm(engine, to);
- ret = ppgtt->switch_mm(ppgtt, req);
- /* The hardware context switch is emitted, but we haven't
- * actually changed the state - so it's probably safe to bail
- * here. Still, let the user know something dangerous has
- * happened.
- */
- if (ret)
- return ret;
- }
-
if (ppgtt)
ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 7481c8e1b5a8..3d18d67d065f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -793,8 +793,6 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
"WA batchbuffer", ee->wa_batchbuffer);
}
- print_error_obj(m, NULL, "Semaphores", error->semaphore);
-
if (error->overlay)
intel_overlay_print_error_state(m, error->overlay);
@@ -903,8 +901,6 @@ void __i915_gpu_state_free(struct kref *error_ref)
kfree(ee->waiters);
}
- i915_error_object_free(error->semaphore);
-
for (i = 0; i < ARRAY_SIZE(error->active_bo); i++)
kfree(error->active_bo[i]);
kfree(error->pinned_bo);
@@ -1116,34 +1112,6 @@ gen8_engine_sync_index(struct intel_engine_cs *engine,
return idx;
}
-static void gen8_record_semaphore_state(struct i915_gpu_state *error,
- struct intel_engine_cs *engine,
- struct drm_i915_error_engine *ee)
-{
- struct drm_i915_private *dev_priv = engine->i915;
- struct intel_engine_cs *to;
- enum intel_engine_id id;
-
- if (!error->semaphore)
- return;
-
- for_each_engine(to, dev_priv, id) {
- int idx;
- u16 signal_offset;
- u32 *tmp;
-
- if (engine == to)
- continue;
-
- signal_offset =
- (GEN8_SIGNAL_OFFSET(engine, id) & (PAGE_SIZE - 1)) / 4;
- tmp = error->semaphore->pages[0];
- idx = gen8_engine_sync_index(engine, to);
-
- ee->semaphore_mboxes[idx] = tmp[signal_offset];
- }
-}
-
static void gen6_record_semaphore_state(struct intel_engine_cs *engine,
struct drm_i915_error_engine *ee)
{
@@ -1218,7 +1186,6 @@ static void error_record_engine_registers(struct i915_gpu_state *error,
if (INTEL_GEN(dev_priv) >= 6) {
ee->rc_psmi = I915_READ(RING_PSMI_CTL(engine->mmio_base));
if (INTEL_GEN(dev_priv) >= 8) {
- gen8_record_semaphore_state(error, engine, ee);
ee->fault_reg = I915_READ(GEN8_RING_FAULT_REG);
} else {
gen6_record_semaphore_state(engine, ee);
@@ -1453,9 +1420,6 @@ static void i915_gem_record_rings(struct drm_i915_private *dev_priv,
struct i915_ggtt *ggtt = &dev_priv->ggtt;
int i;
- error->semaphore =
- i915_error_object_create(dev_priv, dev_priv->semaphore);
-
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct intel_engine_cs *engine = dev_priv->engine[i];
struct drm_i915_error_engine *ee = &error->engine[i];
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 4258f94a1960..db44122c1118 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -364,18 +364,6 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno)
if (HAS_VEBOX(dev_priv))
I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
}
- if (dev_priv->semaphore) {
- struct page *page = i915_vma_first_page(dev_priv->semaphore);
- void *semaphores;
-
- /* Semaphores are in noncoherent memory, flush to be safe */
- semaphores = kmap_atomic(page);
- memset(semaphores + GEN8_SEMAPHORE_OFFSET(engine->id, 0),
- 0, I915_NUM_ENGINES * gen8_semaphore_seqno_size);
- drm_clflush_virt_range(semaphores + GEN8_SEMAPHORE_OFFSET(engine->id, 0),
- I915_NUM_ENGINES * gen8_semaphore_seqno_size);
- kunmap_atomic(semaphores);
- }
intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno);
clear_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted);
diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c
index 12ac270a5f93..c8383c30b142 100644
--- a/drivers/gpu/drm/i915/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/intel_hangcheck.c
@@ -27,13 +27,9 @@
static bool
ipehr_is_semaphore_wait(struct intel_engine_cs *engine, u32 ipehr)
{
- if (INTEL_GEN(engine->i915) >= 8) {
- return (ipehr >> 23) == 0x1c;
- } else {
- ipehr &= ~MI_SEMAPHORE_SYNC_MASK;
- return ipehr == (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE |
- MI_SEMAPHORE_REGISTER);
- }
+ ipehr &= ~MI_SEMAPHORE_SYNC_MASK;
+ return ipehr == (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE |
+ MI_SEMAPHORE_REGISTER);
}
static struct intel_engine_cs *
@@ -41,31 +37,20 @@ semaphore_wait_to_signaller_ring(struct intel_engine_cs *engine, u32 ipehr,
u64 offset)
{
struct drm_i915_private *dev_priv = engine->i915;
+ u32 sync_bits = ipehr & MI_SEMAPHORE_SYNC_MASK;
struct intel_engine_cs *signaller;
enum intel_engine_id id;
- if (INTEL_GEN(dev_priv) >= 8) {
- for_each_engine(signaller, dev_priv, id) {
- if (engine == signaller)
- continue;
-
- if (offset == signaller->semaphore.signal_ggtt[engine->hw_id])
- return signaller;
- }
- } else {
- u32 sync_bits = ipehr & MI_SEMAPHORE_SYNC_MASK;
-
- for_each_engine(signaller, dev_priv, id) {
- if(engine == signaller)
- continue;
+ for_each_engine(signaller, dev_priv, id) {
+ if(engine == signaller)
+ continue;
- if (sync_bits == signaller->semaphore.mbox.wait[engine->hw_id])
- return signaller;
- }
+ if (sync_bits == signaller->semaphore.mbox.wait[engine->hw_id])
+ return signaller;
}
- DRM_DEBUG_DRIVER("No signaller ring found for %s, ipehr 0x%08x, offset 0x%016llx\n",
- engine->name, ipehr, offset);
+ DRM_DEBUG_DRIVER("No signaller ring found for %s, ipehr 0x%08x\n",
+ engine->name, ipehr);
return ERR_PTR(-ENODEV);
}
@@ -135,11 +120,6 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
return NULL;
*seqno = ioread32(vaddr + head + 4) + 1;
- if (INTEL_GEN(dev_priv) >= 8) {
- offset = ioread32(vaddr + head + 12);
- offset <<= 32;
- offset |= ioread32(vaddr + head + 8);
- }
return semaphore_wait_to_signaller_ring(engine, ipehr, offset);
}
@@ -273,7 +253,7 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd)
return ENGINE_WAIT_KICK;
}
- if (INTEL_GEN(dev_priv) >= 6 && tmp & RING_WAIT_SEMAPHORE) {
+ if (IS_GEN(dev_priv, 6, 7) && tmp & RING_WAIT_SEMAPHORE) {
switch (semaphore_passed(engine)) {
default:
return ENGINE_DEAD;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index be98868115bf..5465e708545f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -340,50 +340,6 @@ gen7_render_ring_flush(struct drm_i915_gem_request *req, u32 mode)
return 0;
}
-static int
-gen8_render_ring_flush(struct drm_i915_gem_request *req, u32 mode)
-{
- u32 flags;
- u32 *cs;
-
- cs = intel_ring_begin(req, mode & EMIT_INVALIDATE ? 12 : 6);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
-
- flags = PIPE_CONTROL_CS_STALL;
-
- if (mode & EMIT_FLUSH) {
- flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
- flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
- flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
- flags |= PIPE_CONTROL_FLUSH_ENABLE;
- }
- if (mode & EMIT_INVALIDATE) {
- flags |= PIPE_CONTROL_TLB_INVALIDATE;
- flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE;
- flags |= PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE;
- flags |= PIPE_CONTROL_VF_CACHE_INVALIDATE;
- flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
- flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
- flags |= PIPE_CONTROL_QW_WRITE;
- flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
-
- /* WaCsStallBeforeStateCacheInvalidate:bdw,chv */
- cs = gen8_emit_pipe_control(cs,
- PIPE_CONTROL_CS_STALL |
- PIPE_CONTROL_STALL_AT_SCOREBOARD,
- 0);
- }
-
- cs = gen8_emit_pipe_control(cs, flags,
- i915_ggtt_offset(req->engine->scratch) +
- 2 * CACHELINE_BYTES);
-
- intel_ring_advance(req, cs);
-
- return 0;
-}
-
static void ring_setup_phys_status_page(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
@@ -427,7 +383,6 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
} else if (IS_GEN6(dev_priv)) {
mmio = RING_HWS_PGA_GEN6(engine->mmio_base);
} else {
- /* XXX: gen8 returns to sanity */
mmio = RING_HWS_PGA(engine->mmio_base);
}
@@ -437,13 +392,7 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
I915_WRITE(mmio, engine->status_page.ggtt_offset);
POSTING_READ(mmio);
- /*
- * Flush the TLB for this page
- *
- * FIXME: These two bits have disappeared on gen8, so a question
- * arises: do we still need this and if so how should we go about
- * invalidating the TLB?
- */
+ /* Flush the TLB for this page */
if (IS_GEN(dev_priv, 6, 7)) {
i915_reg_t reg = RING_INSTPM(engine->mmio_base);
@@ -611,8 +560,6 @@ static void reset_ring_common(struct intel_engine_cs *engine,
struct intel_context *ce = &request->ctx->engine[engine->id];
struct i915_hw_ppgtt *ppgtt;
- /* FIXME consider gen8 reset */
-
if (ce->state) {
I915_WRITE(CCID,
i915_ggtt_offset(ce->state) |
@@ -713,62 +660,6 @@ static int init_render_ring(struct intel_engine_cs *engine)
return init_workarounds_ring(engine);
}
-static void render_ring_cleanup(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *dev_priv = engine->i915;
-
- i915_vma_unpin_and_release(&dev_priv->semaphore);
-}
-
-static u32 *gen8_rcs_signal(struct drm_i915_gem_request *req, u32 *cs)
-{
- struct drm_i915_private *dev_priv = req->i915;
- struct intel_engine_cs *waiter;
- enum intel_engine_id id;
-
- for_each_engine(waiter, dev_priv, id) {
- u64 gtt_offset = req->engine->semaphore.signal_ggtt[id];
- if (gtt_offset == MI_SEMAPHORE_SYNC_INVALID)
- continue;
-
- *cs++ = GFX_OP_PIPE_CONTROL(6);
- *cs++ = PIPE_CONTROL_GLOBAL_GTT_IVB | PIPE_CONTROL_QW_WRITE |
- PIPE_CONTROL_CS_STALL;
- *cs++ = lower_32_bits(gtt_offset);
- *cs++ = upper_32_bits(gtt_offset);
- *cs++ = req->global_seqno;
- *cs++ = 0;
- *cs++ = MI_SEMAPHORE_SIGNAL |
- MI_SEMAPHORE_TARGET(waiter->hw_id);
- *cs++ = 0;
- }
-
- return cs;
-}
-
-static u32 *gen8_xcs_signal(struct drm_i915_gem_request *req, u32 *cs)
-{
- struct drm_i915_private *dev_priv = req->i915;
- struct intel_engine_cs *waiter;
- enum intel_engine_id id;
-
- for_each_engine(waiter, dev_priv, id) {
- u64 gtt_offset = req->engine->semaphore.signal_ggtt[id];
- if (gtt_offset == MI_SEMAPHORE_SYNC_INVALID)
- continue;
-
- *cs++ = (MI_FLUSH_DW + 1) | MI_FLUSH_DW_OP_STOREDW;
- *cs++ = lower_32_bits(gtt_offset) | MI_FLUSH_DW_USE_GTT;
- *cs++ = upper_32_bits(gtt_offset);
- *cs++ = req->global_seqno;
- *cs++ = MI_SEMAPHORE_SIGNAL |
- MI_SEMAPHORE_TARGET(waiter->hw_id);
- *cs++ = 0;
- }
-
- return cs;
-}
-
static u32 *gen6_signal(struct drm_i915_gem_request *req, u32 *cs)
{
struct drm_i915_private *dev_priv = req->i915;
@@ -851,70 +742,6 @@ static void gen6_sema_emit_breadcrumb(struct drm_i915_gem_request *req, u32 *cs)
req->engine->semaphore.signal(req, cs));
}
-static void gen8_render_emit_breadcrumb(struct drm_i915_gem_request *req,
- u32 *cs)
-{
- struct intel_engine_cs *engine = req->engine;
-
- if (engine->semaphore.signal)
- cs = engine->semaphore.signal(req, cs);
-
- *cs++ = GFX_OP_PIPE_CONTROL(6);
- *cs++ = PIPE_CONTROL_GLOBAL_GTT_IVB | PIPE_CONTROL_CS_STALL |
- PIPE_CONTROL_QW_WRITE;
- *cs++ = intel_hws_seqno_address(engine);
- *cs++ = 0;
- *cs++ = req->global_seqno;
- /* We're thrashing one dword of HWS. */
- *cs++ = 0;
- *cs++ = MI_USER_INTERRUPT;
- *cs++ = MI_NOOP;
-
- req->tail = intel_ring_offset(req, cs);
- assert_ring_tail_valid(req->ring, req->tail);
-}
-
-static const int gen8_render_emit_breadcrumb_sz = 8;
-
-/**
- * intel_ring_sync - sync the waiter to the signaller on seqno
- *
- * @waiter - ring that is waiting
- * @signaller - ring which has, or will signal
- * @seqno - seqno which the waiter will block on
- */
-
-static int
-gen8_ring_sync_to(struct drm_i915_gem_request *req,
- struct drm_i915_gem_request *signal)
-{
- struct drm_i915_private *dev_priv = req->i915;
- u64 offset = GEN8_WAIT_OFFSET(req->engine, signal->engine->id);
- struct i915_hw_ppgtt *ppgtt;
- u32 *cs;
-
- cs = intel_ring_begin(req, 4);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
-
- *cs++ = MI_SEMAPHORE_WAIT | MI_SEMAPHORE_GLOBAL_GTT |
- MI_SEMAPHORE_SAD_GTE_SDD;
- *cs++ = signal->global_seqno;
- *cs++ = lower_32_bits(offset);
- *cs++ = upper_32_bits(offset);
- intel_ring_advance(req, cs);
-
- /* When the !RCS engines idle waiting upon a semaphore, they lose their
- * pagetables and we must reload them before executing the batch.
- * We do this on the i915_switch_context() following the wait and
- * before the dispatch.
- */
- ppgtt = req->ctx->ppgtt;
- if (ppgtt && req->engine->id != RCS)
- ppgtt->pd_dirty_rings |= intel_engine_flag(req->engine);
- return 0;
-}
-
static int
gen6_ring_sync_to(struct drm_i915_gem_request *req,
struct drm_i915_gem_request *signal)
@@ -1090,25 +917,6 @@ hsw_vebox_irq_disable(struct intel_engine_cs *engine)
gen6_mask_pm_irq(dev_priv, engine->irq_enable_mask);
}
-static void
-gen8_irq_enable(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *dev_priv = engine->i915;
-
- I915_WRITE_IMR(engine,
- ~(engine->irq_enable_mask |
- engine->irq_keep_mask));
- POSTING_READ_FW(RING_IMR(engine->mmio_base));
-}
-
-static void
-gen8_irq_disable(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *dev_priv = engine->i915;
-
- I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
-}
-
static int
i965_emit_bb_start(struct drm_i915_gem_request *req,
u64 offset, u32 length,
@@ -1796,8 +1604,6 @@ static int gen6_bsd_ring_flush(struct drm_i915_gem_request *req, u32 mode)
return PTR_ERR(cs);
cmd = MI_FLUSH_DW;
- if (INTEL_GEN(req->i915) >= 8)
- cmd += 1;
/* We always require a command barrier so that subsequent
* commands, such as breadcrumb interrupts, are strictly ordered
@@ -1817,38 +1623,9 @@ static int gen6_bsd_ring_flush(struct drm_i915_gem_request *req, u32 mode)
*cs++ = cmd;
*cs++ = I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT;
- if (INTEL_GEN(req->i915) >= 8) {
- *cs++ = 0; /* upper addr */
- *cs++ = 0; /* value */
- } else {
- *cs++ = 0;
- *cs++ = MI_NOOP;
- }
- intel_ring_advance(req, cs);
- return 0;
-}
-
-static int
-gen8_emit_bb_start(struct drm_i915_gem_request *req,
- u64 offset, u32 len,
- unsigned int dispatch_flags)
-{
- bool ppgtt = USES_PPGTT(req->i915) &&
- !(dispatch_flags & I915_DISPATCH_SECURE);
- u32 *cs;
-
- cs = intel_ring_begin(req, 4);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
-
- /* FIXME(BDW): Address space and security selectors. */
- *cs++ = MI_BATCH_BUFFER_START_GEN8 | (ppgtt << 8) | (dispatch_flags &
- I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);
- *cs++ = lower_32_bits(offset);
- *cs++ = upper_32_bits(offset);
+ *cs++ = 0;
*cs++ = MI_NOOP;
intel_ring_advance(req, cs);
-
return 0;
}
@@ -1905,8 +1682,6 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req, u32 mode)
return PTR_ERR(cs);
cmd = MI_FLUSH_DW;
- if (INTEL_GEN(req->i915) >= 8)
- cmd += 1;
/* We always require a command barrier so that subsequent
* commands, such as breadcrumb interrupts, are strictly ordered
@@ -1925,13 +1700,8 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req, u32 mode)
cmd |= MI_INVALIDATE_TLB;
*cs++ = cmd;
*cs++ = I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT;
- if (INTEL_GEN(req->i915) >= 8) {
- *cs++ = 0; /* upper addr */
- *cs++ = 0; /* value */
- } else {
- *cs++ = 0;
- *cs++ = MI_NOOP;
- }
+ *cs++ = 0;
+ *cs++ = MI_NOOP;
intel_ring_advance(req, cs);
return 0;
@@ -1940,110 +1710,61 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req, u32 mode)
static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
struct intel_engine_cs *engine)
{
- struct drm_i915_gem_object *obj;
- int ret, i;
+ int i;
if (!i915_modparams.semaphores)
return;
- if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore) {
- struct i915_vma *vma;
-
- obj = i915_gem_object_create(dev_priv, PAGE_SIZE);
- if (IS_ERR(obj))
- goto err;
-
- vma = i915_vma_instance(obj, &dev_priv->ggtt.base, NULL);
- if (IS_ERR(vma))
- goto err_obj;
-
- ret = i915_gem_object_set_to_gtt_domain(obj, false);
- if (ret)
- goto err_obj;
-
- ret = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
- if (ret)
- goto err_obj;
+ GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
+ engine->semaphore.sync_to = gen6_ring_sync_to;
+ engine->semaphore.signal = gen6_signal;
- dev_priv->semaphore = vma;
- }
-
- if (INTEL_GEN(dev_priv) >= 8) {
- u32 offset = i915_ggtt_offset(dev_priv->semaphore);
-
- engine->semaphore.sync_to = gen8_ring_sync_to;
- engine->semaphore.signal = gen8_xcs_signal;
-
- for (i = 0; i < I915_NUM_ENGINES; i++) {
- u32 ring_offset;
-
- if (i != engine->id)
- ring_offset = offset + GEN8_SEMAPHORE_OFFSET(engine->id, i);
- else
- ring_offset = MI_SEMAPHORE_SYNC_INVALID;
-
- engine->semaphore.signal_ggtt[i] = ring_offset;
- }
- } else if (INTEL_GEN(dev_priv) >= 6) {
- engine->semaphore.sync_to = gen6_ring_sync_to;
- engine->semaphore.signal = gen6_signal;
-
- /*
- * The current semaphore is only applied on pre-gen8
- * platform. And there is no VCS2 ring on the pre-gen8
- * platform. So the semaphore between RCS and VCS2 is
- * initialized as INVALID. Gen8 will initialize the
- * sema between VCS2 and RCS later.
- */
- for (i = 0; i < GEN6_NUM_SEMAPHORES; i++) {
- static const struct {
- u32 wait_mbox;
- i915_reg_t mbox_reg;
- } sem_data[GEN6_NUM_SEMAPHORES][GEN6_NUM_SEMAPHORES] = {
- [RCS_HW] = {
- [VCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_RV, .mbox_reg = GEN6_VRSYNC },
- [BCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_RB, .mbox_reg = GEN6_BRSYNC },
- [VECS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_RVE, .mbox_reg = GEN6_VERSYNC },
- },
- [VCS_HW] = {
- [RCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VR, .mbox_reg = GEN6_RVSYNC },
- [BCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VB, .mbox_reg = GEN6_BVSYNC },
- [VECS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VVE, .mbox_reg = GEN6_VEVSYNC },
- },
- [BCS_HW] = {
- [RCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_BR, .mbox_reg = GEN6_RBSYNC },
- [VCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_BV, .mbox_reg = GEN6_VBSYNC },
- [VECS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_BVE, .mbox_reg = GEN6_VEBSYNC },
- },
- [VECS_HW] = {
- [RCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VER, .mbox_reg = GEN6_RVESYNC },
- [VCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VEV, .mbox_reg = GEN6_VVESYNC },
- [BCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VEB, .mbox_reg = GEN6_BVESYNC },
- },
- };
+ /*
+ * The current semaphore is only applied on pre-gen8
+ * platform. And there is no VCS2 ring on the pre-gen8
+ * platform. So the semaphore between RCS and VCS2 is
+ * initialized as INVALID.
+ */
+ for (i = 0; i < GEN6_NUM_SEMAPHORES; i++) {
+ static const struct {
u32 wait_mbox;
i915_reg_t mbox_reg;
+ } sem_data[GEN6_NUM_SEMAPHORES][GEN6_NUM_SEMAPHORES] = {
+ [RCS_HW] = {
+ [VCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_RV, .mbox_reg = GEN6_VRSYNC },
+ [BCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_RB, .mbox_reg = GEN6_BRSYNC },
+ [VECS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_RVE, .mbox_reg = GEN6_VERSYNC },
+ },
+ [VCS_HW] = {
+ [RCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VR, .mbox_reg = GEN6_RVSYNC },
+ [BCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VB, .mbox_reg = GEN6_BVSYNC },
+ [VECS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VVE, .mbox_reg = GEN6_VEVSYNC },
+ },
+ [BCS_HW] = {
+ [RCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_BR, .mbox_reg = GEN6_RBSYNC },
+ [VCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_BV, .mbox_reg = GEN6_VBSYNC },
+ [VECS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_BVE, .mbox_reg = GEN6_VEBSYNC },
+ },
+ [VECS_HW] = {
+ [RCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VER, .mbox_reg = GEN6_RVESYNC },
+ [VCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VEV, .mbox_reg = GEN6_VVESYNC },
+ [BCS_HW] = { .wait_mbox = MI_SEMAPHORE_SYNC_VEB, .mbox_reg = GEN6_BVESYNC },
+ },
+ };
+ u32 wait_mbox;
+ i915_reg_t mbox_reg;
- if (i == engine->hw_id) {
- wait_mbox = MI_SEMAPHORE_SYNC_INVALID;
- mbox_reg = GEN6_NOSYNC;
- } else {
- wait_mbox = sem_data[engine->hw_id][i].wait_mbox;
- mbox_reg = sem_data[engine->hw_id][i].mbox_reg;
- }
-
- engine->semaphore.mbox.wait[i] = wait_mbox;
- engine->semaphore.mbox.signal[i] = mbox_reg;
+ if (i == engine->hw_id) {
+ wait_mbox = MI_SEMAPHORE_SYNC_INVALID;
+ mbox_reg = GEN6_NOSYNC;
+ } else {
+ wait_mbox = sem_data[engine->hw_id][i].wait_mbox;
+ mbox_reg = sem_data[engine->hw_id][i].mbox_reg;
}
- }
-
- return;
-err_obj:
- i915_gem_object_put(obj);
-err:
- DRM_DEBUG_DRIVER("Failed to allocate space for semaphores, disabling\n");
- i915_modparams.semaphores = 0;
+ engine->semaphore.mbox.wait[i] = wait_mbox;
+ engine->semaphore.mbox.signal[i] = mbox_reg;
+ }
}
static void intel_ring_init_irq(struct drm_i915_private *dev_priv,
@@ -2051,11 +1772,7 @@ static void intel_ring_init_irq(struct drm_i915_private *dev_priv,
{
engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << engine->irq_shift;
- if (INTEL_GEN(dev_priv) >= 8) {
- engine->irq_enable = gen8_irq_enable;
- engine->irq_disable = gen8_irq_disable;
- engine->irq_seqno_barrier = gen6_seqno_barrier;
- } else if (INTEL_GEN(dev_priv) >= 6) {
+ if (INTEL_GEN(dev_priv) >= 6) {
engine->irq_enable = gen6_irq_enable;
engine->irq_disable = gen6_irq_disable;
engine->irq_seqno_barrier = gen6_seqno_barrier;
@@ -2090,6 +1807,9 @@ static void gen6_bsd_set_default_submission(struct intel_engine_cs *engine)
static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
struct intel_engine_cs *engine)
{
+ /* gen8+ are only supported with execlists */
+ GEM_BUG_ON(INTEL_GEN(dev_priv) >= 8);
+
intel_ring_init_irq(dev_priv, engine);
intel_ring_init_semaphores(dev_priv, engine);
@@ -2109,20 +1829,14 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
engine->emit_breadcrumb = gen6_sema_emit_breadcrumb;
num_rings = INTEL_INFO(dev_priv)->num_rings - 1;
- if (INTEL_GEN(dev_priv) >= 8) {
- engine->emit_breadcrumb_sz += num_rings * 6;
- } else {
- engine->emit_breadcrumb_sz += num_rings * 3;
- if (num_rings & 1)
- engine->emit_breadcrumb_sz++;
- }
+ engine->emit_breadcrumb_sz += num_rings * 3;
+ if (num_rings & 1)
+ engine->emit_breadcrumb_sz++;
}
engine->set_default_submission = i9xx_set_default_submission;
- if (INTEL_GEN(dev_priv) >= 8)
- engine->emit_bb_start = gen8_emit_bb_start;
- else if (INTEL_GEN(dev_priv) >= 6)
+ if (INTEL_GEN(dev_priv) >= 6)
engine->emit_bb_start = gen6_emit_bb_start;
else if (INTEL_GEN(dev_priv) >= 4)
engine->emit_bb_start = i965_emit_bb_start;
@@ -2142,20 +1856,7 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
if (HAS_L3_DPF(dev_priv))
engine->irq_keep_mask = GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
- if (INTEL_GEN(dev_priv) >= 8) {
- engine->init_context = intel_rcs_ctx_init;
- engine->emit_breadcrumb = gen8_render_emit_breadcrumb;
- engine->emit_breadcrumb_sz = gen8_render_emit_breadcrumb_sz;
- engine->emit_flush = gen8_render_ring_flush;
- if (i915_modparams.semaphores) {
- int num_rings;
-
- engine->semaphore.signal = gen8_rcs_signal;
-
- num_rings = INTEL_INFO(dev_priv)->num_rings - 1;
- engine->emit_breadcrumb_sz += num_rings * 8;
- }
- } else if (INTEL_GEN(dev_priv) >= 6) {
+ if (INTEL_GEN(dev_priv) >= 6) {
engine->init_context = intel_rcs_ctx_init;
engine->emit_flush = gen7_render_ring_flush;
if (IS_GEN6(dev_priv))
@@ -2174,7 +1875,6 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
engine->emit_bb_start = hsw_emit_bb_start;
engine->init_hw = init_render_ring;
- engine->cleanup = render_ring_cleanup;
ret = intel_init_ring_buffer(engine);
if (ret)
@@ -2204,8 +1904,7 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
if (IS_GEN6(dev_priv))
engine->set_default_submission = gen6_bsd_set_default_submission;
engine->emit_flush = gen6_bsd_ring_flush;
- if (INTEL_GEN(dev_priv) < 8)
- engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
+ engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
} else {
engine->mmio_base = BSD_RING_BASE;
engine->emit_flush = bsd_ring_flush;
@@ -2225,8 +1924,7 @@ int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
intel_ring_default_vfuncs(dev_priv, engine);
engine->emit_flush = gen6_ring_flush;
- if (INTEL_GEN(dev_priv) < 8)
- engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
+ engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
return intel_init_ring_buffer(engine);
}
@@ -2238,12 +1936,9 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
intel_ring_default_vfuncs(dev_priv, engine);
engine->emit_flush = gen6_ring_flush;
-
- if (INTEL_GEN(dev_priv) < 8) {
- engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
- engine->irq_enable = hsw_vebox_irq_enable;
- engine->irq_disable = hsw_vebox_irq_disable;
- }
+ engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
+ engine->irq_enable = hsw_vebox_irq_enable;
+ engine->irq_disable = hsw_vebox_irq_disable;
return intel_init_ring_buffer(engine);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0531d939fe1a..0ed5e7bd2363 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -47,16 +47,6 @@ struct intel_hw_status_page {
/* seqno size is actually only a uint32, but since we plan to use MI_FLUSH_DW to
* do the writes, and that must have qw aligned offsets, simply pretend it's 8b.
*/
-#define gen8_semaphore_seqno_size sizeof(uint64_t)
-#define GEN8_SEMAPHORE_OFFSET(__from, __to) \
- (((__from) * I915_NUM_ENGINES + (__to)) * gen8_semaphore_seqno_size)
-#define GEN8_SIGNAL_OFFSET(__ring, to) \
- (dev_priv->semaphore->node.start + \
- GEN8_SEMAPHORE_OFFSET((__ring)->id, (to)))
-#define GEN8_WAIT_OFFSET(__ring, from) \
- (dev_priv->semaphore->node.start + \
- GEN8_SEMAPHORE_OFFSET(from, (__ring)->id))
-
enum intel_engine_hangcheck_action {
ENGINE_IDLE = 0,
ENGINE_WAIT,
@@ -467,18 +457,15 @@ struct intel_engine_cs {
* ie. transpose of f(x, y)
*/
struct {
- union {
#define GEN6_SEMAPHORE_LAST VECS_HW
#define GEN6_NUM_SEMAPHORES (GEN6_SEMAPHORE_LAST + 1)
#define GEN6_SEMAPHORES_MASK GENMASK(GEN6_SEMAPHORE_LAST, 0)
- struct {
- /* our mbox written by others */
- u32 wait[GEN6_NUM_SEMAPHORES];
- /* mboxes this ring signals to */
- i915_reg_t signal[GEN6_NUM_SEMAPHORES];
- } mbox;
- u64 signal_ggtt[I915_NUM_ENGINES];
- };
+ struct {
+ /* our mbox written by others */
+ u32 wait[GEN6_NUM_SEMAPHORES];
+ /* mboxes this ring signals to */
+ i915_reg_t signal[GEN6_NUM_SEMAPHORES];
+ } mbox;
/* AKA wait() */
int (*sync_to)(struct drm_i915_gem_request *req,
--
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] 20+ messages in thread
* [PATCH 6/8] drm/i915: Disable semaphores on Sandybridge
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (3 preceding siblings ...)
2017-11-14 21:30 ` [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+ Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
2017-11-16 12:52 ` Joonas Lahtinen
2017-11-14 21:30 ` [PATCH 7/8] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info Chris Wilson
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 UTC (permalink / raw)
To: intel-gfx
I should have admitted defeat long ago as there has been a rare but
persistent error on Sandybridge where semaphore signaling did not
propagate to the waiter, leading to a GPU hang.
With the work on fence signaling for v4.9, the impact of using CPU driven
signaling was greatly reduced wrt to the latency of GPU semaphores,
though without logical rings support, the benefit of reordering work to
avoid bubbles is not realised (i.e. as it stands fence signaling is just
a slower, more costly version of HW semaphores; but works more
consistently). As a rough indicator of the difference,
with semaphores:
Sequential (3 engines, 1 processes): average 5.470us per cycle [expected 4.988us]
w/o semaphores:
Sequential (3 engines, 1 processes): average 15.771us per cycle [expected 4.923us]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54226
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 93bfcaccad6c..01e54c93520b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4997,20 +4997,12 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
{
- if (INTEL_GEN(dev_priv) < 6)
- return false;
-
- /* TODO: make semaphores and Execlists play nicely together */
- if (HAS_EXECLISTS(dev_priv))
+ if (!IS_GEN7(dev_priv))
return false;
if (value >= 0)
return value;
- /* Enable semaphores on SNB when IO remapping is off */
- if (IS_GEN6(dev_priv) && intel_vtd_active())
- return false;
-
return true;
}
--
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] 20+ messages in thread
* [PATCH 7/8] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (4 preceding siblings ...)
2017-11-14 21:30 ` [PATCH 6/8] drm/i915: Disable semaphores on Sandybridge Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
2017-11-16 13:01 ` Joonas Lahtinen
2017-11-14 21:30 ` [PATCH 8/8] drm/i915: Remove i915.semaphores modparam Chris Wilson
` (3 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 UTC (permalink / raw)
To: intel-gfx
As the semaphores is just part of the engine, include it with the
general pretty printer universally used for debugging.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 32 --------------------------------
drivers/gpu/drm/i915/intel_engine_cs.c | 9 +++++++++
2 files changed, 9 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c2375aaecb9a..7637cec50049 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3222,37 +3222,6 @@ static int i915_shrinker_info(struct seq_file *m, void *unused)
return 0;
}
-static int i915_semaphore_status(struct seq_file *m, void *unused)
-{
- struct drm_i915_private *dev_priv = node_to_i915(m->private);
- struct drm_device *dev = &dev_priv->drm;
- struct intel_engine_cs *engine;
- int num_rings = INTEL_INFO(dev_priv)->num_rings;
- enum intel_engine_id id;
- int j, ret;
-
- if (!i915_modparams.semaphores) {
- seq_puts(m, "Semaphores are disabled\n");
- return 0;
- }
-
- ret = mutex_lock_interruptible(&dev->struct_mutex);
- if (ret)
- return ret;
- intel_runtime_pm_get(dev_priv);
-
- seq_puts(m, " Last signal:");
- for_each_engine(engine, dev_priv, id)
- for (j = 0; j < num_rings; j++)
- seq_printf(m, "0x%08x\n",
- I915_READ(engine->semaphore.mbox.signal[j]));
- seq_putc(m, '\n');
-
- intel_runtime_pm_put(dev_priv);
- mutex_unlock(&dev->struct_mutex);
- return 0;
-}
-
static int i915_shared_dplls_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4732,7 +4701,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_display_info", i915_display_info, 0},
{"i915_engine_info", i915_engine_info, 0},
{"i915_shrinker_info", i915_shrinker_info, 0},
- {"i915_semaphore_status", i915_semaphore_status, 0},
{"i915_shared_dplls_info", i915_shared_dplls_info, 0},
{"i915_dp_mst_info", i915_dp_mst_info, 0},
{"i915_wa_registers", i915_wa_registers, 0},
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index db44122c1118..c34e52d44f76 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1715,6 +1715,15 @@ void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m)
I915_READ(RING_MI_MODE(engine->mmio_base)),
I915_READ(RING_MI_MODE(engine->mmio_base)) & (MODE_IDLE) ? " [idle]" : "");
}
+ if (i915_modparams.semaphores) {
+ drm_printf(m, "\tSYNC_0: 0x%08x\n",
+ I915_READ(RING_SYNC_0(engine->mmio_base)));
+ drm_printf(m, "\tSYNC_1: 0x%08x\n",
+ I915_READ(RING_SYNC_1(engine->mmio_base)));
+ if (HAS_VEBOX(dev_priv))
+ drm_printf(m, "\tSYNC_2: 0x%08x\n",
+ I915_READ(RING_SYNC_2(engine->mmio_base)));
+ }
rcu_read_unlock();
--
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] 20+ messages in thread
* [PATCH 8/8] drm/i915: Remove i915.semaphores modparam
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (5 preceding siblings ...)
2017-11-14 21:30 ` [PATCH 7/8] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info Chris Wilson
@ 2017-11-14 21:30 ` Chris Wilson
2017-11-16 13:13 ` Joonas Lahtinen
2017-11-14 21:58 ` ✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer Patchwork
` (2 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-14 21:30 UTC (permalink / raw)
To: intel-gfx
Having disabled the broken semaphores on Sandybridge, there is no need
for a modparam any more, so remove it in favour of a simple
HAS_LEGACY_SEMAPHORES() guard.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 7 +------
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_gem.c | 11 -----------
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_params.c | 4 ----
drivers/gpu/drm/i915/i915_params.h | 1 -
drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
8 files changed, 7 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index abf1073bee88..2b97ff9a0e4e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -321,7 +321,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
value = USES_PPGTT(dev_priv);
break;
case I915_PARAM_HAS_SEMAPHORES:
- value = i915_modparams.semaphores;
+ value = HAS_LEGACY_SEMAPHORES(dev_priv);
break;
case I915_PARAM_HAS_SECURE_BATCHES:
value = capable(CAP_SYS_ADMIN);
@@ -1061,11 +1061,6 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
i915_modparams.enable_ppgtt);
DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915_modparams.enable_ppgtt);
- i915_modparams.semaphores =
- intel_sanitize_semaphores(dev_priv, i915_modparams.semaphores);
- DRM_DEBUG_DRIVER("use GPU semaphores? %s\n",
- yesno(i915_modparams.semaphores));
-
intel_uc_sanitize_options(dev_priv);
intel_gvt_sanitize_options(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4172b67a3db0..3796dfc9358d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3140,6 +3140,8 @@ intel_info(const struct drm_i915_private *dev_priv)
#define HAS_BLT(dev_priv) HAS_ENGINE(dev_priv, BCS)
#define HAS_VEBOX(dev_priv) HAS_ENGINE(dev_priv, VECS)
+#define HAS_LEGACY_SEMAPHORES(dev_priv) IS_GEN7(dev_priv)
+
#define HAS_LLC(dev_priv) ((dev_priv)->info.has_llc)
#define HAS_SNOOP(dev_priv) ((dev_priv)->info.has_snoop)
#define HAS_EDRAM(dev_priv) (!!((dev_priv)->edram_cap & EDRAM_ENABLED))
@@ -3303,8 +3305,6 @@ intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
int enable_ppgtt);
-bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value);
-
/* i915_drv.c */
void __printf(3, 4)
__i915_printk(struct drm_i915_private *dev_priv, const char *level,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 01e54c93520b..aaf9a05b757e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4995,17 +4995,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
return ret;
}
-bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
-{
- if (!IS_GEN7(dev_priv))
- return false;
-
- if (value >= 0)
- return value;
-
- return true;
-}
-
static int __intel_engines_record_defaults(struct drm_i915_private *i915)
{
struct i915_gem_context *ctx;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 0704d9af261b..6ca56e482d79 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -574,7 +574,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
enum intel_engine_id id;
const int num_rings =
/* Use an extended w/a on gen7 if signalling from other rings */
- (i915_modparams.semaphores && IS_GEN7(dev_priv)) ?
+ (HAS_LEGACY_SEMAPHORES(dev_priv) && IS_GEN7(dev_priv)) ?
INTEL_INFO(dev_priv)->num_rings - 1 :
0;
int len;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index d61c1787c164..3328147b4863 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -46,10 +46,6 @@ i915_param_named_unsafe(panel_ignore_lid, int, 0600,
"Override lid status (0=autodetect, 1=autodetect disabled [default], "
"-1=force lid closed, -2=force lid open)");
-i915_param_named_unsafe(semaphores, int, 0400,
- "Use semaphores for inter-ring sync "
- "(default: -1 (use per-chip defaults))");
-
i915_param_named_unsafe(enable_rc6, int, 0400,
"Enable power-saving render C-state 6. "
"Different stages can be selected via bitmask values "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 0aef3d7178eb..8321bd86cba5 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -31,7 +31,6 @@
param(char *, vbt_firmware, NULL) \
param(int, modeset, -1) \
param(int, panel_ignore_lid, 1) \
- param(int, semaphores, -1) \
param(int, lvds_channel_mode, 0) \
param(int, panel_use_ssc, -1) \
param(int, vbt_sdvo_panel_type, -1) \
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index c34e52d44f76..6803e248ccb8 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1715,7 +1715,7 @@ void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m)
I915_READ(RING_MI_MODE(engine->mmio_base)),
I915_READ(RING_MI_MODE(engine->mmio_base)) & (MODE_IDLE) ? " [idle]" : "");
}
- if (i915_modparams.semaphores) {
+ if (HAS_LEGACY_SEMAPHORES(dev_priv)) {
drm_printf(m, "\tSYNC_0: 0x%08x\n",
I915_READ(RING_SYNC_0(engine->mmio_base)));
drm_printf(m, "\tSYNC_1: 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5465e708545f..bfa11a84e476 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1712,7 +1712,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
{
int i;
- if (!i915_modparams.semaphores)
+ if (!HAS_LEGACY_SEMAPHORES(dev_priv))
return;
GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
@@ -1823,7 +1823,7 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
engine->emit_breadcrumb = i9xx_emit_breadcrumb;
engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz;
- if (i915_modparams.semaphores) {
+ if (HAS_LEGACY_SEMAPHORES(dev_priv)) {
int num_rings;
engine->emit_breadcrumb = gen6_sema_emit_breadcrumb;
--
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] 20+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (6 preceding siblings ...)
2017-11-14 21:30 ` [PATCH 8/8] drm/i915: Remove i915.semaphores modparam Chris Wilson
@ 2017-11-14 21:58 ` Patchwork
2017-11-15 0:49 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-15 12:14 ` [PATCH 1/8] " Mika Kuoppala
9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-14 21:58 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer
URL : https://patchwork.freedesktop.org/series/33830/
State : success
== Summary ==
Series 33830v1 series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer
https://patchwork.freedesktop.org/api/1.0/series/33830/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
incomplete -> PASS (fi-snb-2520m) fdo#103713
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:447s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:454s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:378s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:544s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:273s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:508s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:505s
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:495s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:495s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:428s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:263s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:541s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:427s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:440s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:430s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:484s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:457s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:483s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:529s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:475s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:535s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:569s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:457s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:545s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:567s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:519s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:499s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:460s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:560s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:420s
Blacklisted hosts:
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:533s
fi-cnl-y total:238 pass:213 dwarn:0 dfail:0 fail:0 skip:24
fi-glk-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:490s
c46476e24d6432b5792ef63596a985848d122d50 drm-tip: 2017y-11m-14d-17h-03m-32s UTC integration manifest
ac6b6b22d6f7 drm/i915: Remove i915.semaphores modparam
e06003755318 drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info
fa9569a8802c drm/i915: Disable semaphores on Sandybridge
e72956d9b9f4 drm/i915: Remove obsolete ringbuffer emission for gen8+
24ff3bfeb616 drm/i915: Remove i915.enable_execlists module parameter
a8d9e6d57b70 drm/i915: Automatic i915_switch_context for legacy
1825aaf2e6f1 drm/i915: Make request's wait-for-space explicit
291d85046393 drm/i915/selftests: Increase size for mock ringbuffer
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7130/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (7 preceding siblings ...)
2017-11-14 21:58 ` ✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer Patchwork
@ 2017-11-15 0:49 ` Patchwork
2017-11-15 12:14 ` [PATCH 1/8] " Mika Kuoppala
9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-15 0:49 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer
URL : https://patchwork.freedesktop.org/series/33830/
State : success
== Summary ==
Test perf:
Subgroup polling:
fail -> PASS (shard-hsw) fdo#102252
Test kms_busy:
Subgroup extended-modeset-hang-oldfb-with-reset-render-c:
pass -> DMESG-WARN (shard-hsw) fdo#102249 +1
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-varying-size:
fail -> PASS (shard-hsw)
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
shard-hsw total:2584 pass:1470 dwarn:4 dfail:1 fail:10 skip:1099 time:9447s
Blacklisted hosts:
shard-apl total:2506 pass:1569 dwarn:1 dfail:0 fail:17 skip:917 time:12173s
shard-kbl total:2517 pass:1665 dwarn:8 dfail:2 fail:17 skip:823 time:10141s
shard-snb total:2548 pass:1185 dwarn:1 dfail:1 fail:13 skip:1346 time:7491s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7130/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
` (8 preceding siblings ...)
2017-11-15 0:49 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-11-15 12:14 ` Mika Kuoppala
9 siblings, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-11-15 12:14 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> We don't actually emit any commands into the ringbuffer, so we set it
> very small. However, an upcoming change centralises the wait-for-space
> into i915_gem_request_alloc() and that imposes a minimum size upon all
> ringbuffers (mock or real) of MIN_SPACE_FOR_ADD_REQUEST. Grow the
> mock ringbuffer such that we allocate a single page for the struct+buffer,
> satisfying the new condition without wasting too much space.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/selftests/mock_engine.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
> index 0aafa8a105b8..55c0e2c15782 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
> @@ -124,9 +124,11 @@ static void mock_submit_request(struct drm_i915_gem_request *request)
>
> static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
> {
> - const unsigned long sz = roundup_pow_of_two(sizeof(struct intel_ring));
> + const unsigned long sz = PAGE_SIZE / 2;
> struct intel_ring *ring;
>
> + BUILD_BUG_ON(MIN_SPACE_FOR_ADD_REQUEST > sz);
> +
> ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> if (!ring)
> return NULL;
> --
> 2.15.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/8] drm/i915: Make request's wait-for-space explicit
[not found] ` <87shdfps4u.fsf@gaia.fi.intel.com>
@ 2017-11-15 14:07 ` Chris Wilson
2017-11-15 14:30 ` Mika Kuoppala
0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-15 14:07 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2017-11-15 13:51:13)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > 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;
> >
>
> We lose the wrap handling with this. What guarantees
> at this point, that we dont need to wrap?
We don't care. We wait for sufficient space to wrap, but we don't need
to advance/wrap the ring pointer here as we are not emitting a packet.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/8] drm/i915: Make request's wait-for-space explicit
2017-11-15 14:07 ` Chris Wilson
@ 2017-11-15 14:30 ` Mika Kuoppala
0 siblings, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-11-15 14:30 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2017-11-15 13:51:13)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>>
>> > 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;
>> >
>>
>> We lose the wrap handling with this. What guarantees
>> at this point, that we dont need to wrap?
>
> We don't care. We wait for sufficient space to wrap, but we don't need
> to advance/wrap the ring pointer here as we are not emitting a packet.
Ok, and the request we alloced for, will do the needed ring_begin
and handle the wrap.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/8] drm/i915: Disable semaphores on Sandybridge
2017-11-14 21:30 ` [PATCH 6/8] drm/i915: Disable semaphores on Sandybridge Chris Wilson
@ 2017-11-16 12:52 ` Joonas Lahtinen
0 siblings, 0 replies; 20+ messages in thread
From: Joonas Lahtinen @ 2017-11-16 12:52 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Tue, 2017-11-14 at 21:30 +0000, Chris Wilson wrote:
> I should have admitted defeat long ago as there has been a rare but
> persistent error on Sandybridge where semaphore signaling did not
> propagate to the waiter, leading to a GPU hang.
>
> With the work on fence signaling for v4.9, the impact of using CPU driven
> signaling was greatly reduced wrt to the latency of GPU semaphores,
> though without logical rings support, the benefit of reordering work to
> avoid bubbles is not realised (i.e. as it stands fence signaling is just
> a slower, more costly version of HW semaphores; but works more
> consistently). As a rough indicator of the difference,
>
> with semaphores:
> Sequential (3 engines, 1 processes): average 5.470us per cycle [expected 4.988us]
>
> w/o semaphores:
> Sequential (3 engines, 1 processes): average 15.771us per cycle [expected 4.923us]
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54226
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Might be worth mentioning that the bug casually has 110 duplicates...
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/8] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info
2017-11-14 21:30 ` [PATCH 7/8] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info Chris Wilson
@ 2017-11-16 13:01 ` Joonas Lahtinen
0 siblings, 0 replies; 20+ messages in thread
From: Joonas Lahtinen @ 2017-11-16 13:01 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Tue, 2017-11-14 at 21:30 +0000, Chris Wilson wrote:
> As the semaphores is just part of the engine, include it with the
> general pretty printer universally used for debugging.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 8/8] drm/i915: Remove i915.semaphores modparam
2017-11-14 21:30 ` [PATCH 8/8] drm/i915: Remove i915.semaphores modparam Chris Wilson
@ 2017-11-16 13:13 ` Joonas Lahtinen
0 siblings, 0 replies; 20+ messages in thread
From: Joonas Lahtinen @ 2017-11-16 13:13 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Tue, 2017-11-14 at 21:30 +0000, Chris Wilson wrote:
> Having disabled the broken semaphores on Sandybridge, there is no need
> for a modparam any more, so remove it in favour of a simple
> HAS_LEGACY_SEMAPHORES() guard.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
This is reported through the uAPI, so UMDs will know to cope if they
care.
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy
2017-11-14 21:30 ` [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy Chris Wilson
@ 2017-11-16 14:00 ` Mika Kuoppala
2017-11-16 14:44 ` Chris Wilson
0 siblings, 1 reply; 20+ messages in thread
From: Mika Kuoppala @ 2017-11-16 14:00 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> 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 a7979b74ce21..d885cf0d2943 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5043,7 +5043,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;
> +
Ok as we don't emit anything with request alloc you can switch the
order.
> /* 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);
..so why this check. What could emit and why the early return?
And also more in generally, we do drop some error return patchs
due to removing switch_context(). Is there a risk of ignoring
and not propagating some error code we did actually catch
at i915_switch_context()?
-Mika
> + }
> +
> /* 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy
2017-11-16 14:00 ` Mika Kuoppala
@ 2017-11-16 14:44 ` Chris Wilson
2017-11-17 11:58 ` Mika Kuoppala
0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-11-16 14:44 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2017-11-16 14:00:13)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > 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 a7979b74ce21..d885cf0d2943 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -5043,7 +5043,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;
> > +
>
> Ok as we don't emit anything with request alloc you can switch the
> order.
> > /* 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);
>
> ..so why this check. What could emit and why the early return?
We check just in case request_alloc did emit some commands, such as
MI_SET_CONTEXT but failed to emit LRI to update the ppgtt. In such a
case, we would have updated engine->legacy_active_context and so we must
commit the request to keep the SW bookkeeping in track with the HW.
So we assume that if we did emit a command, we need to submit.
> And also more in generally, we do drop some error return patchs
> due to removing switch_context(). Is there a risk of ignoring
> and not propagating some error code we did actually catch
> at i915_switch_context()?
No. The error returns still propagate through request_alloc.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy
2017-11-16 14:44 ` Chris Wilson
@ 2017-11-17 11:58 ` Mika Kuoppala
0 siblings, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-11-17 11:58 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2017-11-16 14:00:13)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>>
>> > 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 a7979b74ce21..d885cf0d2943 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem.c
>> > @@ -5043,7 +5043,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;
>> > +
>>
>> Ok as we don't emit anything with request alloc you can switch the
>> order.
>> > /* 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);
>>
>> ..so why this check. What could emit and why the early return?
>
> We check just in case request_alloc did emit some commands, such as
> MI_SET_CONTEXT but failed to emit LRI to update the ppgtt. In such a
> case, we would have updated engine->legacy_active_context and so we must
> commit the request to keep the SW bookkeeping in track with the HW.
>
> So we assume that if we did emit a command, we need to submit.
It is a bit embarrasing for me that the actual MI_SET_CONTEXT
that does the emit is in this same patch, below :P
Oh well, thanks for explanation it makes sense now.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+
2017-11-14 21:30 ` [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+ Chris Wilson
@ 2017-11-17 13:29 ` Mika Kuoppala
0 siblings, 0 replies; 20+ messages in thread
From: Mika Kuoppala @ 2017-11-17 13:29 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Since removing the module parameter to force selection of ringbuffer
> emission for gen8, the code is defunct. Remove it.
>
> To put the difference into perspective, a couple of microbenchmarks
> (bdw i7-5557u, 20170324):
> ring execlists
> exec continuous nops on all rings: 1.491us 2.223us
> exec sequential nops on each ring: 12.508us 53.682us
> single nop + sync: 9.272us 30.291us
>
> vblank_mode=0 glxgears: ~11000fps ~9000fps
>
> Since the earlier submission, gen8 ringbuffer submission has fallen
> further and further behind in features. So while ringbuffer may hold the
> throughput crown, in terms of interactive latency, execlists is much
> better. Alas, we have no convenient metrics for such, other than
> demonstrating things we can do with execlists but can not using
> legacy ringbuffer submission.
>
> We have made a few improvements to lowlevel execlists throughput,
> and ringbuffer currently panics on boot! (bdw i7-5557u, 20171026):
>
> ring execlists
> exec continuous nops on all rings: n/a 1.921us
> exec sequential nops on each ring: n/a 44.621us
> single nop + sync: n/a 21.953us
>
> vblank_mode=0 glxgears: n/a ~18500fps
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=87725
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Once-upon-a-time-Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
lgtm but found one thing more we can throw out:
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 45835d83dfbf..873439abc415 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -37,8 +37,6 @@
* Resource Streamer, is 66944 bytes, which rounds to 17 pages.
*/
#define HSW_CXT_TOTAL_SIZE (17 * PAGE_SIZE)
-/* Same as Haswell, but 72064 bytes now. */
-#define GEN8_CXT_TOTAL_SIZE (18 * PAGE_SIZE)
#define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
#define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
@@ -164,9 +162,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
case 9:
return GEN9_LR_CONTEXT_RENDER_SIZE;
case 8:
- return i915_modparams.enable_execlists ?
- GEN8_LR_CONTEXT_RENDER_SIZE :
- GEN8_CXT_TOTAL_SIZE;
+ return GEN8_LR_CONTEXT_RENDER_SIZE;
case 7:
With that included,
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-11-17 13:29 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-14 21:30 [PATCH 1/8] drm/i915/selftests: Increase size for mock ringbuffer Chris Wilson
2017-11-14 21:30 ` [PATCH 2/8] drm/i915: Make request's wait-for-space explicit Chris Wilson
[not found] ` <87shdfps4u.fsf@gaia.fi.intel.com>
2017-11-15 14:07 ` Chris Wilson
2017-11-15 14:30 ` Mika Kuoppala
2017-11-14 21:30 ` [PATCH 3/8] drm/i915: Automatic i915_switch_context for legacy Chris Wilson
2017-11-16 14:00 ` Mika Kuoppala
2017-11-16 14:44 ` Chris Wilson
2017-11-17 11:58 ` Mika Kuoppala
2017-11-14 21:30 ` [PATCH 4/8] drm/i915: Remove i915.enable_execlists module parameter Chris Wilson
2017-11-14 21:30 ` [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+ Chris Wilson
2017-11-17 13:29 ` Mika Kuoppala
2017-11-14 21:30 ` [PATCH 6/8] drm/i915: Disable semaphores on Sandybridge Chris Wilson
2017-11-16 12:52 ` Joonas Lahtinen
2017-11-14 21:30 ` [PATCH 7/8] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info Chris Wilson
2017-11-16 13:01 ` Joonas Lahtinen
2017-11-14 21:30 ` [PATCH 8/8] drm/i915: Remove i915.semaphores modparam Chris Wilson
2017-11-16 13:13 ` Joonas Lahtinen
2017-11-14 21:58 ` ✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915/selftests: Increase size for mock ringbuffer Patchwork
2017-11-15 0:49 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-15 12:14 ` [PATCH 1/8] " Mika Kuoppala
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.