* [PATCH 1/5] drm/i915: execlist request keeps ptr/ref to gem_request
2014-11-12 10:53 [PATCH 0/5] drm/i915: Untangle execlist tracking Nick Hoath
@ 2014-11-12 10:53 ` Nick Hoath
2014-11-12 10:53 ` [PATCH 2/5] drm/i915: Removed duplicate members from submit_request Nick Hoath
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Nick Hoath @ 2014-11-12 10:53 UTC (permalink / raw)
To: intel-gfx
Add a reference and pointer from the execlist queue item to the associated
gem request. For execlist requests that don't have a request, create one
as a placeholder.
This patchset requires John Harrison's "Replace seqno values with request structures"
patchset.
Issue: VIZ-4274
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 31 +++++++++++++++++++++++++------
drivers/gpu/drm/i915/intel_lrc.h | 5 ++++-
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c8b3827..593471f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -521,6 +521,7 @@ static void execlists_free_request_task(struct work_struct *work)
mutex_lock(&dev->struct_mutex);
i915_gem_context_unreference(req->ctx);
+ i915_gem_request_unreference(req->request);
mutex_unlock(&dev->struct_mutex);
kfree(req);
@@ -528,7 +529,8 @@ static void execlists_free_request_task(struct work_struct *work)
static int execlists_context_queue(struct intel_engine_cs *ring,
struct intel_context *to,
- u32 tail)
+ u32 tail,
+ struct drm_i915_gem_request *request)
{
struct intel_ctx_submit_request *req = NULL, *cursor;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
@@ -544,6 +546,22 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
req->tail = tail;
INIT_WORK(&req->work, execlists_free_request_task);
+ if(!request)
+ {
+ /*
+ * If there isn't a request associated with this submission,
+ * create one as a temporary holder.
+ */
+ WARN(1, "execlist context submission without request");
+ request = kzalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+ request->ctx = to;
+ request->ring = ring;
+ }
+ req->request = request;
+ i915_gem_request_reference(request);
+
intel_runtime_pm_get(dev_priv);
spin_lock_irqsave(&ring->execlist_lock, flags);
@@ -778,7 +796,8 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
* on a queue waiting for the ELSP to be ready to accept a new context submission. At that
* point, the tail *inside* the context is updated and the ELSP written to.
*/
-void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
+void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
+ struct drm_i915_gem_request *request)
{
struct intel_engine_cs *ring = ringbuf->ring;
struct intel_context *ctx = ringbuf->FIXME_lrc_ctx;
@@ -788,7 +807,7 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
if (intel_ring_stopped(ring))
return;
- execlists_context_queue(ring, ctx, ringbuf->tail);
+ execlists_context_queue(ring, ctx, ringbuf->tail, request);
}
static int logical_ring_alloc_request(struct intel_engine_cs *ring,
@@ -876,7 +895,7 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
return ret;
/* Force the context submission in case we have been skipping it */
- intel_logical_ring_advance_and_submit(ringbuf);
+ intel_logical_ring_advance_and_submit(ringbuf, NULL);
/* With GEM the hangcheck timer should kick us out of the loop,
* leaving it early runs the risk of corrupting GEM state (due
@@ -1221,7 +1240,7 @@ static void gen8_set_seqno(struct intel_engine_cs *ring, u32 seqno)
intel_write_status_page(ring, I915_GEM_HWS_INDEX, seqno);
}
-static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
+static int gen8_emit_request(struct intel_ringbuffer *ringbuf, struct drm_i915_gem_request *request)
{
struct intel_engine_cs *ring = ringbuf->ring;
u32 cmd;
@@ -1243,7 +1262,7 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
i915_gem_request_get_seqno(ring->outstanding_lazy_request));
intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
intel_logical_ring_emit(ringbuf, MI_NOOP);
- intel_logical_ring_advance_and_submit(ringbuf);
+ intel_logical_ring_advance_and_submit(ringbuf, request);
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 33c3b4b..6f81669 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -37,7 +37,8 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
int intel_logical_rings_init(struct drm_device *dev);
int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf);
-void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf);
+void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
+ struct drm_i915_gem_request *request);
/**
* intel_logical_ring_advance() - advance the ringbuffer tail
* @ringbuf: Ringbuffer to advance.
@@ -107,6 +108,8 @@ struct intel_ctx_submit_request {
struct work_struct work;
int elsp_submitted;
+
+ struct drm_i915_gem_request *request;
};
void intel_execlists_handle_ctx_events(struct intel_engine_cs *ring);
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] drm/i915: Removed duplicate members from submit_request
2014-11-12 10:53 [PATCH 0/5] drm/i915: Untangle execlist tracking Nick Hoath
2014-11-12 10:53 ` [PATCH 1/5] drm/i915: execlist request keeps ptr/ref to gem_request Nick Hoath
@ 2014-11-12 10:53 ` Nick Hoath
2014-11-12 10:53 ` [PATCH 3/5] drm/i915: Remove FIXME_lrc_ctx backpointer Nick Hoath
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Nick Hoath @ 2014-11-12 10:53 UTC (permalink / raw)
To: intel-gfx
Where there were duplicate variables for the tail, context and ring (engine)
in the gem request and the execlist queue item, use the one from the request
and remove the duplicate from the execlist queue item.
Issue: VIZ-4274
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 21 +++++++++------------
drivers/gpu/drm/i915/intel_lrc.h | 4 ----
4 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0d6af1c..45da79e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1891,11 +1891,11 @@ static int i915_execlists(struct seq_file *m, void *data)
if (head_req) {
struct drm_i915_gem_object *ctx_obj;
- ctx_obj = head_req->ctx->engine[ring_id].state;
+ ctx_obj = head_req->request->ctx->engine[ring_id].state;
seq_printf(m, "\tHead request id: %u\n",
intel_execlists_ctx_id(ctx_obj));
seq_printf(m, "\tHead request tail: %u\n",
- head_req->tail);
+ head_req->request->tail);
}
seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 860c296..e5f521f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2704,7 +2704,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
execlist_link);
list_del(&submit_req->execlist_link);
intel_runtime_pm_put(dev_priv);
- i915_gem_context_unreference(submit_req->ctx);
+ i915_gem_context_unreference(submit_req->request->ctx);
kfree(submit_req);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 593471f..0e2e33b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -396,7 +396,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
execlist_link) {
if (!req0) {
req0 = cursor;
- } else if (req0->ctx == cursor->ctx) {
+ } else if (req0->request->ctx == cursor->request->ctx) {
/* Same ctx: ignore first request, as second request
* will update tail past first request's workload */
cursor->elsp_submitted = req0->elsp_submitted;
@@ -411,9 +411,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
WARN_ON(req1 && req1->elsp_submitted);
- execlists_submit_contexts(ring, req0->ctx, req0->tail,
- req1 ? req1->ctx : NULL,
- req1 ? req1->tail : 0);
+ execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
+ req1 ? req1->request->ctx : NULL,
+ req1 ? req1->request->tail : 0);
req0->elsp_submitted++;
if (req1)
@@ -434,7 +434,7 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
if (head_req != NULL) {
struct drm_i915_gem_object *ctx_obj =
- head_req->ctx->engine[ring->id].state;
+ head_req->request->ctx->engine[ring->id].state;
if (intel_execlists_ctx_id(ctx_obj) == request_id) {
WARN(head_req->elsp_submitted == 0,
"Never submitted head request\n");
@@ -514,13 +514,13 @@ static void execlists_free_request_task(struct work_struct *work)
{
struct intel_ctx_submit_request *req =
container_of(work, struct intel_ctx_submit_request, work);
- struct drm_device *dev = req->ring->dev;
+ struct drm_device *dev = req->request->ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
intel_runtime_pm_put(dev_priv);
mutex_lock(&dev->struct_mutex);
- i915_gem_context_unreference(req->ctx);
+ i915_gem_context_unreference(req->request->ctx);
i915_gem_request_unreference(req->request);
mutex_unlock(&dev->struct_mutex);
@@ -540,10 +540,6 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
req = kzalloc(sizeof(*req), GFP_KERNEL);
if (req == NULL)
return -ENOMEM;
- req->ctx = to;
- i915_gem_context_reference(req->ctx);
- req->ring = ring;
- req->tail = tail;
INIT_WORK(&req->work, execlists_free_request_task);
if(!request)
@@ -561,6 +557,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
}
req->request = request;
i915_gem_request_reference(request);
+ i915_gem_context_reference(req->request->ctx);
intel_runtime_pm_get(dev_priv);
@@ -577,7 +574,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
struct intel_ctx_submit_request,
execlist_link);
- if (to == tail_req->ctx) {
+ if (to == tail_req->request->ctx) {
WARN(tail_req->elsp_submitted != 0,
"More than 2 already-submitted reqs queued\n");
list_del(&tail_req->execlist_link);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 6f81669..76141ce 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -100,10 +100,6 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
* All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
*/
struct intel_ctx_submit_request {
- struct intel_context *ctx;
- struct intel_engine_cs *ring;
- u32 tail;
-
struct list_head execlist_link;
struct work_struct work;
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] drm/i915: Remove FIXME_lrc_ctx backpointer
2014-11-12 10:53 [PATCH 0/5] drm/i915: Untangle execlist tracking Nick Hoath
2014-11-12 10:53 ` [PATCH 1/5] drm/i915: execlist request keeps ptr/ref to gem_request Nick Hoath
2014-11-12 10:53 ` [PATCH 2/5] drm/i915: Removed duplicate members from submit_request Nick Hoath
@ 2014-11-12 10:53 ` Nick Hoath
2014-11-12 10:53 ` [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request Nick Hoath
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Nick Hoath @ 2014-11-12 10:53 UTC (permalink / raw)
To: intel-gfx
The first pass implementation of execlists required a backpointer to the context to be held
in the intel_ringbuffer. However the context pointer is available higher in the call stack.
Remove the backpointer from the ring buffer structure and instead pass it down through the
call stack.
v2: Integrate this changeset with the removal of duplicate request/execlist queue item members.
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Issue: VIZ-4268
---
drivers/gpu/drm/i915/i915_gem.c | 7 ++--
drivers/gpu/drm/i915/intel_lrc.c | 67 +++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_lrc.h | 8 +++-
drivers/gpu/drm/i915/intel_ringbuffer.h | 12 +++---
4 files changed, 56 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e5f521f..bd5a1e2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2451,8 +2451,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
return -ENOMEM;
if (i915.enable_execlists) {
- struct intel_context *ctx = request->ctx;
- ringbuf = ctx->engine[ring->id].ringbuf;
+ ringbuf = request->ctx->engine[ring->id].ringbuf;
} else
ringbuf = ring->buffer;
@@ -2465,7 +2464,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
* what.
*/
if (i915.enable_execlists) {
- ret = logical_ring_flush_all_caches(ringbuf);
+ ret = logical_ring_flush_all_caches(ringbuf, request->ctx);
if (ret)
return ret;
} else {
@@ -2487,7 +2486,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
request_ring_position = intel_ring_get_tail(ringbuf);
if (i915.enable_execlists) {
- ret = ring->emit_request(ringbuf, request);
+ ret = ring->emit_request(ringbuf, request->ctx, request);
if (ret)
return ret;
} else {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0e2e33b..4bd9572 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -555,6 +555,10 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
request->ctx = to;
request->ring = ring;
}
+ else
+ {
+ WARN_ON(to != request->ctx);
+ }
req->request = request;
i915_gem_request_reference(request);
i915_gem_context_reference(req->request->ctx);
@@ -591,7 +595,8 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
return 0;
}
-static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
+static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx)
{
struct intel_engine_cs *ring = ringbuf->ring;
uint32_t flush_domains;
@@ -601,7 +606,8 @@ static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
if (ring->gpu_caches_dirty)
flush_domains = I915_GEM_GPU_DOMAINS;
- ret = ring->emit_flush(ringbuf, I915_GEM_GPU_DOMAINS, flush_domains);
+ ret = ring->emit_flush(ringbuf, ctx,
+ I915_GEM_GPU_DOMAINS, flush_domains);
if (ret)
return ret;
@@ -610,6 +616,7 @@ static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
}
static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
struct list_head *vmas)
{
struct intel_engine_cs *ring = ringbuf->ring;
@@ -637,7 +644,7 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
/* Unconditionally invalidate gpu caches and ensure that we do flush
* any residual writes from the previous batch.
*/
- return logical_ring_invalidate_all_caches(ringbuf);
+ return logical_ring_invalidate_all_caches(ringbuf, ctx);
}
/**
@@ -717,13 +724,13 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
return -EINVAL;
}
- ret = execlists_move_to_gpu(ringbuf, vmas);
+ ret = execlists_move_to_gpu(ringbuf, ctx, vmas);
if (ret)
return ret;
if (ring == &dev_priv->ring[RCS] &&
instp_mode != dev_priv->relative_constants_mode) {
- ret = intel_logical_ring_begin(ringbuf, 4);
+ ret = intel_logical_ring_begin(ringbuf, ctx, 4);
if (ret)
return ret;
@@ -736,7 +743,7 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
dev_priv->relative_constants_mode = instp_mode;
}
- ret = ring->emit_bb_start(ringbuf, exec_start, flags);
+ ret = ring->emit_bb_start(ringbuf, ctx, exec_start, flags);
if (ret)
return ret;
@@ -768,7 +775,8 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring)
I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING));
}
-int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
+int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx)
{
struct intel_engine_cs *ring = ringbuf->ring;
int ret;
@@ -776,7 +784,7 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
if (!ring->gpu_caches_dirty)
return 0;
- ret = ring->emit_flush(ringbuf, 0, I915_GEM_GPU_DOMAINS);
+ ret = ring->emit_flush(ringbuf, ctx, 0, I915_GEM_GPU_DOMAINS);
if (ret)
return ret;
@@ -794,10 +802,10 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf)
* point, the tail *inside* the context is updated and the ELSP written to.
*/
void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
struct drm_i915_gem_request *request)
{
struct intel_engine_cs *ring = ringbuf->ring;
- struct intel_context *ctx = ringbuf->FIXME_lrc_ctx;
intel_logical_ring_advance(ringbuf);
@@ -879,6 +887,7 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
}
static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
int bytes)
{
struct intel_engine_cs *ring = ringbuf->ring;
@@ -892,7 +901,7 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
return ret;
/* Force the context submission in case we have been skipping it */
- intel_logical_ring_advance_and_submit(ringbuf, NULL);
+ intel_logical_ring_advance_and_submit(ringbuf, ctx, NULL);
/* With GEM the hangcheck timer should kick us out of the loop,
* leaving it early runs the risk of corrupting GEM state (due
@@ -930,13 +939,14 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
return ret;
}
-static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf)
+static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx)
{
uint32_t __iomem *virt;
int rem = ringbuf->size - ringbuf->tail;
if (ringbuf->space < rem) {
- int ret = logical_ring_wait_for_space(ringbuf, rem);
+ int ret = logical_ring_wait_for_space(ringbuf, ctx, rem);
if (ret)
return ret;
@@ -953,18 +963,19 @@ static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf)
return 0;
}
-static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, int bytes)
+static int logical_ring_prepare(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx, int bytes)
{
int ret;
if (unlikely(ringbuf->tail + bytes > ringbuf->effective_size)) {
- ret = logical_ring_wrap_buffer(ringbuf);
+ ret = logical_ring_wrap_buffer(ringbuf, ctx);
if (unlikely(ret))
return ret;
}
if (unlikely(ringbuf->space < bytes)) {
- ret = logical_ring_wait_for_space(ringbuf, bytes);
+ ret = logical_ring_wait_for_space(ringbuf, ctx, bytes);
if (unlikely(ret))
return ret;
}
@@ -985,7 +996,8 @@ static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, int bytes)
*
* Return: non-zero if the ringbuffer is not ready to be written to.
*/
-int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords)
+int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx, int num_dwords)
{
struct intel_engine_cs *ring = ringbuf->ring;
struct drm_device *dev = ring->dev;
@@ -997,12 +1009,12 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords)
if (ret)
return ret;
- ret = logical_ring_prepare(ringbuf, num_dwords * sizeof(uint32_t));
+ ret = logical_ring_prepare(ringbuf, ctx, num_dwords * sizeof(uint32_t));
if (ret)
return ret;
/* Preallocate the olr before touching the ring */
- ret = logical_ring_alloc_request(ring, ringbuf->FIXME_lrc_ctx);
+ ret = logical_ring_alloc_request(ring, ctx);
if (ret)
return ret;
@@ -1095,12 +1107,13 @@ static int gen8_init_render_ring(struct intel_engine_cs *ring)
}
static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
u64 offset, unsigned flags)
{
bool ppgtt = !(flags & I915_DISPATCH_SECURE);
int ret;
- ret = intel_logical_ring_begin(ringbuf, 4);
+ ret = intel_logical_ring_begin(ringbuf, ctx, 4);
if (ret)
return ret;
@@ -1148,6 +1161,7 @@ static void gen8_logical_ring_put_irq(struct intel_engine_cs *ring)
}
static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
u32 invalidate_domains,
u32 unused)
{
@@ -1157,7 +1171,7 @@ static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
uint32_t cmd;
int ret;
- ret = intel_logical_ring_begin(ringbuf, 4);
+ ret = intel_logical_ring_begin(ringbuf, ctx, 4);
if (ret)
return ret;
@@ -1186,6 +1200,7 @@ static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
}
static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
u32 invalidate_domains,
u32 flush_domains)
{
@@ -1212,7 +1227,7 @@ static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
}
- ret = intel_logical_ring_begin(ringbuf, 6);
+ ret = intel_logical_ring_begin(ringbuf, ctx, 6);
if (ret)
return ret;
@@ -1237,13 +1252,15 @@ static void gen8_set_seqno(struct intel_engine_cs *ring, u32 seqno)
intel_write_status_page(ring, I915_GEM_HWS_INDEX, seqno);
}
-static int gen8_emit_request(struct intel_ringbuffer *ringbuf, struct drm_i915_gem_request *request)
+static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
+ struct drm_i915_gem_request *request)
{
struct intel_engine_cs *ring = ringbuf->ring;
u32 cmd;
int ret;
- ret = intel_logical_ring_begin(ringbuf, 6);
+ ret = intel_logical_ring_begin(ringbuf, ctx, 6);
if (ret)
return ret;
@@ -1259,7 +1276,7 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf, struct drm_i915_g
i915_gem_request_get_seqno(ring->outstanding_lazy_request));
intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
intel_logical_ring_emit(ringbuf, MI_NOOP);
- intel_logical_ring_advance_and_submit(ringbuf, request);
+ intel_logical_ring_advance_and_submit(ringbuf, ctx, request);
return 0;
}
@@ -1536,6 +1553,7 @@ int intel_lr_context_render_state_init(struct intel_engine_cs *ring,
return 0;
ret = ring->emit_bb_start(ringbuf,
+ ctx,
so.ggtt_offset,
I915_DISPATCH_SECURE);
if (ret)
@@ -1785,7 +1803,6 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
}
ringbuf->ring = ring;
- ringbuf->FIXME_lrc_ctx = ctx;
ringbuf->size = 32 * PAGE_SIZE;
ringbuf->effective_size = ringbuf->size;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 76141ce..bc6ff0d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -36,8 +36,10 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring);
void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
int intel_logical_rings_init(struct drm_device *dev);
-int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf);
+int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx);
void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
struct drm_i915_gem_request *request);
/**
* intel_logical_ring_advance() - advance the ringbuffer tail
@@ -60,7 +62,9 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
ringbuf->tail += 4;
}
-int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords);
+int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
+ int num_dwords);
/* Logical Ring Contexts */
int intel_lr_context_render_state_init(struct intel_engine_cs *ring,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b669f68..6acc254 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -99,13 +99,6 @@ struct intel_ringbuffer {
struct intel_engine_cs *ring;
- /*
- * FIXME: This backpointer is an artifact of the history of how the
- * execlist patches came into being. It will get removed once the basic
- * code has landed.
- */
- struct intel_context *FIXME_lrc_ctx;
-
u32 head;
u32 tail;
int space;
@@ -123,6 +116,8 @@ struct intel_ringbuffer {
u32 last_retired_head;
};
+struct intel_context;
+
struct intel_engine_cs {
const char *name;
enum intel_ring_id {
@@ -239,11 +234,14 @@ struct intel_engine_cs {
u8 next_context_status_buffer;
u32 irq_keep_mask; /* bitmask for interrupts that should not be masked */
int (*emit_request)(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
struct drm_i915_gem_request *request);
int (*emit_flush)(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
u32 invalidate_domains,
u32 flush_domains);
int (*emit_bb_start)(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx,
u64 offset, unsigned flags);
/**
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
2014-11-12 10:53 [PATCH 0/5] drm/i915: Untangle execlist tracking Nick Hoath
` (2 preceding siblings ...)
2014-11-12 10:53 ` [PATCH 3/5] drm/i915: Remove FIXME_lrc_ctx backpointer Nick Hoath
@ 2014-11-12 10:53 ` Nick Hoath
2014-11-12 11:24 ` Chris Wilson
2014-11-12 10:53 ` [PATCH 5/5] drm/i915: Change workaround execlist submission to use gem requests Nick Hoath
2014-12-10 16:28 ` [PATCH 0/5] drm/i915: Untangle execlist tracking Daniel Vetter
5 siblings, 1 reply; 10+ messages in thread
From: Nick Hoath @ 2014-11-12 10:53 UTC (permalink / raw)
To: intel-gfx
Move all remaining elements that were unique to execlists queue items
in to the associated request.
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Issue: VIZ-4274
---
drivers/gpu/drm/i915/i915_debugfs.c | 8 +++----
drivers/gpu/drm/i915/i915_drv.h | 22 +++++++++++++++++
drivers/gpu/drm/i915/i915_gem.c | 6 ++---
drivers/gpu/drm/i915/intel_lrc.c | 47 +++++++++++++++++--------------------
drivers/gpu/drm/i915/intel_lrc.h | 28 ----------------------
5 files changed, 50 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 45da79e..9ce9a02 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1851,7 +1851,7 @@ static int i915_execlists(struct seq_file *m, void *data)
intel_runtime_pm_get(dev_priv);
for_each_ring(ring, dev_priv, ring_id) {
- struct intel_ctx_submit_request *head_req = NULL;
+ struct drm_i915_gem_request *head_req = NULL;
int count = 0;
unsigned long flags;
@@ -1884,18 +1884,18 @@ static int i915_execlists(struct seq_file *m, void *data)
list_for_each(cursor, &ring->execlist_queue)
count++;
head_req = list_first_entry_or_null(&ring->execlist_queue,
- struct intel_ctx_submit_request, execlist_link);
+ struct drm_i915_gem_request, execlist_link);
spin_unlock_irqrestore(&ring->execlist_lock, flags);
seq_printf(m, "\t%d requests in queue\n", count);
if (head_req) {
struct drm_i915_gem_object *ctx_obj;
- ctx_obj = head_req->request->ctx->engine[ring_id].state;
+ ctx_obj = head_req->ctx->engine[ring_id].state;
seq_printf(m, "\tHead request id: %u\n",
intel_execlists_ctx_id(ctx_obj));
seq_printf(m, "\tHead request tail: %u\n",
- head_req->request->tail);
+ head_req->tail);
}
seq_putc(m, '\n');
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index afa9c35..0fe238c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2027,6 +2027,28 @@ struct drm_i915_gem_request {
struct list_head free_list;
uint32_t uniq;
+
+ /**
+ * The ELSP only accepts two elements at a time, so we queue context/tail
+ * pairs on a given queue (ring->execlist_queue) until the hardware is
+ * available. The queue serves a double purpose: we also use it to keep track
+ * of the up to 2 contexts currently in the hardware (usually one in execution
+ * and the other queued up by the GPU): We only remove elements from the head
+ * of the queue when the hardware informs us that an element has been
+ * completed.
+ *
+ * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
+ */
+
+ /** Execlist link in the submission queue.*/
+ struct list_head execlist_link;
+
+ /** Execlists workqueue for processing this request in a bottom half */
+ struct work_struct work;
+
+ /** Execlists no. of times this request has been sent to the ELSP */
+ int elsp_submitted;
+
};
void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bd5a1e2..4d2d2e5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2696,14 +2696,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
}
while (!list_empty(&ring->execlist_queue)) {
- struct intel_ctx_submit_request *submit_req;
+ struct drm_i915_gem_request *submit_req;
submit_req = list_first_entry(&ring->execlist_queue,
- struct intel_ctx_submit_request,
+ struct drm_i915_gem_request,
execlist_link);
list_del(&submit_req->execlist_link);
intel_runtime_pm_put(dev_priv);
- i915_gem_context_unreference(submit_req->request->ctx);
+ i915_gem_context_unreference(submit_req->ctx);
kfree(submit_req);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4bd9572..b6ec012 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -382,8 +382,8 @@ static void execlists_submit_contexts(struct intel_engine_cs *ring,
static void execlists_context_unqueue(struct intel_engine_cs *ring)
{
- struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
- struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
+ struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
+ struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
assert_spin_locked(&ring->execlist_lock);
@@ -396,7 +396,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
execlist_link) {
if (!req0) {
req0 = cursor;
- } else if (req0->request->ctx == cursor->request->ctx) {
+ } else if (req0->ctx == cursor->ctx) {
/* Same ctx: ignore first request, as second request
* will update tail past first request's workload */
cursor->elsp_submitted = req0->elsp_submitted;
@@ -411,9 +411,9 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
WARN_ON(req1 && req1->elsp_submitted);
- execlists_submit_contexts(ring, req0->request->ctx, req0->request->tail,
- req1 ? req1->request->ctx : NULL,
- req1 ? req1->request->tail : 0);
+ execlists_submit_contexts(ring, req0->ctx, req0->tail,
+ req1 ? req1->ctx : NULL,
+ req1 ? req1->tail : 0);
req0->elsp_submitted++;
if (req1)
@@ -424,17 +424,17 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
u32 request_id)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
- struct intel_ctx_submit_request *head_req;
+ struct drm_i915_gem_request *head_req;
assert_spin_locked(&ring->execlist_lock);
head_req = list_first_entry_or_null(&ring->execlist_queue,
- struct intel_ctx_submit_request,
+ struct drm_i915_gem_request,
execlist_link);
if (head_req != NULL) {
struct drm_i915_gem_object *ctx_obj =
- head_req->request->ctx->engine[ring->id].state;
+ head_req->ctx->engine[ring->id].state;
if (intel_execlists_ctx_id(ctx_obj) == request_id) {
WARN(head_req->elsp_submitted == 0,
"Never submitted head request\n");
@@ -512,16 +512,16 @@ void intel_execlists_handle_ctx_events(struct intel_engine_cs *ring)
static void execlists_free_request_task(struct work_struct *work)
{
- struct intel_ctx_submit_request *req =
- container_of(work, struct intel_ctx_submit_request, work);
- struct drm_device *dev = req->request->ring->dev;
+ struct drm_i915_gem_request *req =
+ container_of(work, struct drm_i915_gem_request, work);
+ struct drm_device *dev = req->ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
intel_runtime_pm_put(dev_priv);
mutex_lock(&dev->struct_mutex);
- i915_gem_context_unreference(req->request->ctx);
- i915_gem_request_unreference(req->request);
+ i915_gem_context_unreference(req->ctx);
+ i915_gem_request_unreference(req);
mutex_unlock(&dev->struct_mutex);
kfree(req);
@@ -532,16 +532,11 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
u32 tail,
struct drm_i915_gem_request *request)
{
- struct intel_ctx_submit_request *req = NULL, *cursor;
+ struct drm_i915_gem_request *cursor;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
unsigned long flags;
int num_elements = 0;
- req = kzalloc(sizeof(*req), GFP_KERNEL);
- if (req == NULL)
- return -ENOMEM;
- INIT_WORK(&req->work, execlists_free_request_task);
-
if(!request)
{
/*
@@ -559,9 +554,9 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
{
WARN_ON(to != request->ctx);
}
- req->request = request;
+ INIT_WORK(&request->work, execlists_free_request_task);
i915_gem_request_reference(request);
- i915_gem_context_reference(req->request->ctx);
+ i915_gem_context_reference(request->ctx);
intel_runtime_pm_get(dev_priv);
@@ -572,13 +567,13 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
break;
if (num_elements > 2) {
- struct intel_ctx_submit_request *tail_req;
+ struct drm_i915_gem_request *tail_req;
tail_req = list_last_entry(&ring->execlist_queue,
- struct intel_ctx_submit_request,
+ struct drm_i915_gem_request,
execlist_link);
- if (to == tail_req->request->ctx) {
+ if (to == tail_req->ctx) {
WARN(tail_req->elsp_submitted != 0,
"More than 2 already-submitted reqs queued\n");
list_del(&tail_req->execlist_link);
@@ -586,7 +581,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
}
}
- list_add_tail(&req->execlist_link, &ring->execlist_queue);
+ list_add_tail(&request->execlist_link, &ring->execlist_queue);
if (num_elements == 0)
execlists_context_unqueue(ring);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index bc6ff0d..94fcd5c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -84,34 +84,6 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
u64 exec_start, u32 flags);
u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
-/**
- * struct intel_ctx_submit_request - queued context submission request
- * @ctx: Context to submit to the ELSP.
- * @ring: Engine to submit it to.
- * @tail: how far in the context's ringbuffer this request goes to.
- * @execlist_link: link in the submission queue.
- * @work: workqueue for processing this request in a bottom half.
- * @elsp_submitted: no. of times this request has been sent to the ELSP.
- *
- * The ELSP only accepts two elements at a time, so we queue context/tail
- * pairs on a given queue (ring->execlist_queue) until the hardware is
- * available. The queue serves a double purpose: we also use it to keep track
- * of the up to 2 contexts currently in the hardware (usually one in execution
- * and the other queued up by the GPU): We only remove elements from the head
- * of the queue when the hardware informs us that an element has been
- * completed.
- *
- * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
- */
-struct intel_ctx_submit_request {
- struct list_head execlist_link;
- struct work_struct work;
-
- int elsp_submitted;
-
- struct drm_i915_gem_request *request;
-};
-
void intel_execlists_handle_ctx_events(struct intel_engine_cs *ring);
#endif /* _INTEL_LRC_H_ */
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
2014-11-12 10:53 ` [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request Nick Hoath
@ 2014-11-12 11:24 ` Chris Wilson
2014-11-12 12:13 ` Nick Hoath
0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2014-11-12 11:24 UTC (permalink / raw)
To: Nick Hoath; +Cc: intel-gfx
On Wed, Nov 12, 2014 at 10:53:26AM +0000, Nick Hoath wrote:
seq_putc(m, '\n');
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index afa9c35..0fe238c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2027,6 +2027,28 @@ struct drm_i915_gem_request {
> struct list_head free_list;
>
> uint32_t uniq;
> +
> + /**
> + * The ELSP only accepts two elements at a time, so we queue context/tail
> + * pairs on a given queue (ring->execlist_queue) until the hardware is
> + * available. The queue serves a double purpose: we also use it to keep track
> + * of the up to 2 contexts currently in the hardware (usually one in execution
> + * and the other queued up by the GPU): We only remove elements from the head
> + * of the queue when the hardware informs us that an element has been
> + * completed.
> + *
> + * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
> + */
> +
> + /** Execlist link in the submission queue.*/
> + struct list_head execlist_link;
This is redundant. The request should only be one of the pending or active
lists at any time.
> + /** Execlists workqueue for processing this request in a bottom half */
> + struct work_struct work;
For what purpose? This is not needed.
> + /** Execlists no. of times this request has been sent to the ELSP */
> + int elsp_submitted;
A request can only be submitted exactly once at any time. This
bookkeeping is not part of the request.
Still not detangled I am afraid.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
2014-11-12 11:24 ` Chris Wilson
@ 2014-11-12 12:13 ` Nick Hoath
2014-12-10 16:25 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Nick Hoath @ 2014-11-12 12:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org
On 12/11/2014 11:24, Chris Wilson wrote:
> On Wed, Nov 12, 2014 at 10:53:26AM +0000, Nick Hoath wrote:
> seq_putc(m, '\n');
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index afa9c35..0fe238c 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2027,6 +2027,28 @@ struct drm_i915_gem_request {
>> struct list_head free_list;
>>
>> uint32_t uniq;
>> +
>> + /**
>> + * The ELSP only accepts two elements at a time, so we queue context/tail
>> + * pairs on a given queue (ring->execlist_queue) until the hardware is
>> + * available. The queue serves a double purpose: we also use it to keep track
>> + * of the up to 2 contexts currently in the hardware (usually one in execution
>> + * and the other queued up by the GPU): We only remove elements from the head
>> + * of the queue when the hardware informs us that an element has been
>> + * completed.
>> + *
>> + * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
>> + */
>> +
>> + /** Execlist link in the submission queue.*/
>> + struct list_head execlist_link;
>
> This is redundant. The request should only be one of the pending or active
> lists at any time.
>
This is used by the pending execlist requests list owned by the
intel_engine_cs. The request isn't in both the active and pending
execlist engine lists.
>> + /** Execlists workqueue for processing this request in a bottom half */
>> + struct work_struct work;
>
> For what purpose? This is not needed.
This worker is currently used to free up execlist requests. This goes
away when Thomas Daniel's patchset is merged.
I have spotted a bug in the cleanup handler with the merged
requests/execlists cleanup though.
>
>> + /** Execlists no. of times this request has been sent to the ELSP */
>> + int elsp_submitted;
>
> A request can only be submitted exactly once at any time. This
> bookkeeping is not part of the request.
This is a refcount to preserve the request if it has been resubmitted
due to preemption or TDR, due to a race condition between the HW
finishing with the item and the cleanup/resubmission. Have a look at
e1fee72c2ea2e9c0c6e6743d32a6832f21337d6c which contains a much better
description of why this exists.
>
> Still not detangled I am afraid.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
2014-11-12 12:13 ` Nick Hoath
@ 2014-12-10 16:25 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2014-12-10 16:25 UTC (permalink / raw)
To: Nick Hoath; +Cc: intel-gfx@lists.freedesktop.org
On Wed, Nov 12, 2014 at 12:13:03PM +0000, Nick Hoath wrote:
> On 12/11/2014 11:24, Chris Wilson wrote:
> >On Wed, Nov 12, 2014 at 10:53:26AM +0000, Nick Hoath wrote:
> > seq_putc(m, '\n');
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>index afa9c35..0fe238c 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -2027,6 +2027,28 @@ struct drm_i915_gem_request {
> >> struct list_head free_list;
> >>
> >> uint32_t uniq;
> >>+
> >>+ /**
> >>+ * The ELSP only accepts two elements at a time, so we queue context/tail
> >>+ * pairs on a given queue (ring->execlist_queue) until the hardware is
> >>+ * available. The queue serves a double purpose: we also use it to keep track
> >>+ * of the up to 2 contexts currently in the hardware (usually one in execution
> >>+ * and the other queued up by the GPU): We only remove elements from the head
> >>+ * of the queue when the hardware informs us that an element has been
> >>+ * completed.
> >>+ *
> >>+ * All accesses to the queue are mediated by a spinlock (ring->execlist_lock).
> >>+ */
> >>+
> >>+ /** Execlist link in the submission queue.*/
> >>+ struct list_head execlist_link;
> >
> >This is redundant. The request should only be one of the pending or active
> >lists at any time.
> >
> This is used by the pending execlist requests list owned by the
> intel_engine_cs. The request isn't in both the active and pending execlist
> engine lists.
I guess we'll eventually need to subsume this into the scheduler's list of
ctxs. Or reuse it there. Imo ok for now, might just need a rename
longterm.
> >>+ /** Execlists workqueue for processing this request in a bottom half */
> >>+ struct work_struct work;
> >
> >For what purpose? This is not needed.
> This worker is currently used to free up execlist requests. This goes away
> when Thomas Daniel's patchset is merged.
> I have spotted a bug in the cleanup handler with the merged
> requests/execlists cleanup though.
I guess this will drop out on a rebase now that everything has landed?
> >>+ /** Execlists no. of times this request has been sent to the ELSP */
> >>+ int elsp_submitted;
> >
> >A request can only be submitted exactly once at any time. This
> >bookkeeping is not part of the request.
> This is a refcount to preserve the request if it has been resubmitted due to
> preemption or TDR, due to a race condition between the HW finishing with the
> item and the cleanup/resubmission. Have a look at
> e1fee72c2ea2e9c0c6e6743d32a6832f21337d6c which contains a much better
> description of why this exists.
Yeah this one is still a bit crazy but guess that's just how it is ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] drm/i915: Change workaround execlist submission to use gem requests.
2014-11-12 10:53 [PATCH 0/5] drm/i915: Untangle execlist tracking Nick Hoath
` (3 preceding siblings ...)
2014-11-12 10:53 ` [PATCH 4/5] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request Nick Hoath
@ 2014-11-12 10:53 ` Nick Hoath
2014-12-10 16:28 ` [PATCH 0/5] drm/i915: Untangle execlist tracking Daniel Vetter
5 siblings, 0 replies; 10+ messages in thread
From: Nick Hoath @ 2014-11-12 10:53 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Issue: VIZ-4274
---
drivers/gpu/drm/i915/intel_lrc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b6ec012..f3f1428 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1030,11 +1030,11 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
return 0;
ring->gpu_caches_dirty = true;
- ret = logical_ring_flush_all_caches(ringbuf);
+ ret = logical_ring_flush_all_caches(ringbuf, ctx);
if (ret)
return ret;
- ret = intel_logical_ring_begin(ringbuf, w->count * 2 + 2);
+ ret = intel_logical_ring_begin(ringbuf, ctx, w->count * 2 + 2);
if (ret)
return ret;
@@ -1048,7 +1048,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
intel_logical_ring_advance(ringbuf);
ring->gpu_caches_dirty = true;
- ret = logical_ring_flush_all_caches(ringbuf);
+ ret = logical_ring_flush_all_caches(ringbuf, ctx);
if (ret)
return ret;
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/5] drm/i915: Untangle execlist tracking
2014-11-12 10:53 [PATCH 0/5] drm/i915: Untangle execlist tracking Nick Hoath
` (4 preceding siblings ...)
2014-11-12 10:53 ` [PATCH 5/5] drm/i915: Change workaround execlist submission to use gem requests Nick Hoath
@ 2014-12-10 16:28 ` Daniel Vetter
5 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2014-12-10 16:28 UTC (permalink / raw)
To: Nick Hoath; +Cc: intel-gfx
On Wed, Nov 12, 2014 at 10:53:22AM +0000, Nick Hoath wrote:
> This patchset merges execlist queue items in to gem requests. It does this by
> using the reference count added by John Harrison's "Replace seqno values with
> request structures" patchset to ensure that the gem request is available for
> the whole execlist submission lifespan.
>
>
> v2: merge intel_ctx_submit_request and drm_i915_gem_request, rebase changes &
> add cover letter
Ok, sorry for the long delay - I got a bit distracted and also thought it
would be better to get John's and Thomas' patches in first since this is
based upon them.
But that's now out of the way and I think this is definitely a good step
into the right direction. So can you please rebase and resend so that we
can do the review&merge dance?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread