* [RFC 0/7] Convert parts of intel_lrc.c to requests
@ 2015-07-03 14:09 Mika Kuoppala
2015-07-03 14:09 ` [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
` (7 more replies)
0 siblings, 8 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Hi,
Reader might think that I did this for only mark few elsps
done deeper in callchain. But the reason is that I wanted
to test more uniqueness guarantees for generation of context ids
that is passed for hardware. And found out that the current
code structure strips out all the request/context very early on.
So here is my take to not lose the request that I have found
valuable in skl debugging.
-Mika
Mika Kuoppala (7):
drm/i915: Convert execlist_submit_contexts() for requests
drm/i915: Convert execlists_update_context() for requests
drm/i915: Assign request ringbuf before pin
drm/i915: Convert intel_lr_context_pin() for requests
drm/i915: Convert execlists_elsp_writ() for requests
drm/i915: Convert execlists_ctx_descriptor() for requests
drm/i915: Mark elsps submitted when they are pushed to hw
drivers/gpu/drm/i915/i915_gem.c | 8 +--
drivers/gpu/drm/i915/intel_lrc.c | 117 +++++++++++++++++----------------------
drivers/gpu/drm/i915/intel_lrc.h | 3 +-
3 files changed, 56 insertions(+), 72 deletions(-)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-07 16:49 ` Yu Dai
2015-07-03 14:09 ` [PATCH 2/7] drm/i915: Convert execlists_update_context() " Mika Kuoppala
` (6 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Pass around requests to carry context deeper in callchain.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9b928ab..583210d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -358,29 +358,29 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
return 0;
}
-static void execlists_submit_contexts(struct intel_engine_cs *ring,
- struct intel_context *to0, u32 tail0,
- struct intel_context *to1, u32 tail1)
+static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
+ struct drm_i915_gem_request *rq1)
{
- struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
+ struct intel_engine_cs *ring = rq0->ring;
+ struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
struct drm_i915_gem_object *ctx_obj1 = NULL;
- struct intel_ringbuffer *ringbuf1 = NULL;
BUG_ON(!ctx_obj0);
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
- WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
+ WARN_ON(!i915_gem_obj_is_pinned(rq0->ringbuf->obj));
- execlists_update_context(ctx_obj0, ringbuf0->obj, to0->ppgtt, tail0);
+ execlists_update_context(ctx_obj1, rq0->ringbuf->obj,
+ rq0->ctx->ppgtt, rq0->tail);
+
+ if (rq1) {
+ ctx_obj1 = rq1->ctx->engine[ring->id].state;
- if (to1) {
- ringbuf1 = to1->engine[ring->id].ringbuf;
- ctx_obj1 = to1->engine[ring->id].state;
BUG_ON(!ctx_obj1);
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
- WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
+ WARN_ON(!i915_gem_obj_is_pinned(rq1->ringbuf->obj));
- execlists_update_context(ctx_obj1, ringbuf1->obj, to1->ppgtt, tail1);
+ execlists_update_context(ctx_obj1, rq1->ringbuf->obj,
+ rq1->ctx->ppgtt, rq1->tail);
}
execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
@@ -443,9 +443,7 @@ 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_requests(req0, req1);
req0->elsp_submitted++;
if (req1)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/7] drm/i915: Convert execlists_update_context() for requests
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
2015-07-03 14:09 ` [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-03 14:09 ` [PATCH 3/7] drm/i915: Assign request ringbuf before pin Mika Kuoppala
` (5 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Pass around requests to carry context deeper in callchain.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 583210d..4139eb6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -329,19 +329,24 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
spin_unlock(&dev_priv->uncore.lock);
}
-static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
- struct drm_i915_gem_object *ring_obj,
- struct i915_hw_ppgtt *ppgtt,
- u32 tail)
+static int execlists_update_context(struct drm_i915_gem_request *rq)
{
+ struct intel_engine_cs *ring = rq->ring;
+ struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+ struct drm_i915_gem_object *rb_obj = rq->ringbuf->obj;
struct page *page;
uint32_t *reg_state;
+ BUG_ON(!ctx_obj);
+ WARN_ON(!i915_gem_obj_is_pinned(ctx_obj));
+ WARN_ON(!i915_gem_obj_is_pinned(rb_obj));
+
page = i915_gem_object_get_page(ctx_obj, 1);
reg_state = kmap_atomic(page);
- reg_state[CTX_RING_TAIL+1] = tail;
- reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(ring_obj);
+ reg_state[CTX_RING_TAIL+1] = rq->tail;
+ reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(rb_obj);
/* True PPGTT with dynamic page allocation: update PDP registers and
* point the unallocated PDPs to the scratch page
@@ -365,22 +370,11 @@ static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
struct drm_i915_gem_object *ctx_obj1 = NULL;
- BUG_ON(!ctx_obj0);
- WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
- WARN_ON(!i915_gem_obj_is_pinned(rq0->ringbuf->obj));
-
- execlists_update_context(ctx_obj1, rq0->ringbuf->obj,
- rq0->ctx->ppgtt, rq0->tail);
+ execlists_update_context(rq0);
if (rq1) {
+ execlists_update_context(rq1);
ctx_obj1 = rq1->ctx->engine[ring->id].state;
-
- BUG_ON(!ctx_obj1);
- WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
- WARN_ON(!i915_gem_obj_is_pinned(rq1->ringbuf->obj));
-
- execlists_update_context(ctx_obj1, rq1->ringbuf->obj,
- rq1->ctx->ppgtt, rq1->tail);
}
execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/7] drm/i915: Assign request ringbuf before pin
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
2015-07-03 14:09 ` [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
2015-07-03 14:09 ` [PATCH 2/7] drm/i915: Convert execlists_update_context() " Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-03 14:11 ` Chris Wilson
2015-07-03 14:09 ` [PATCH 4/7] drm/i915: Convert intel_lr_context_pin() for requests Mika Kuoppala
` (4 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
In preparation to make intel_lr_context_pin|unpin to accept
requests, assign ringbuf into request before we call the pinning.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4139eb6..ab8a98c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -633,14 +633,16 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
{
int ret;
+ request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
+
if (request->ctx != request->ring->default_context) {
ret = intel_lr_context_pin(request->ring, request->ctx);
- if (ret)
+ if (ret) {
+ request->ringbuf = NULL;
return ret;
+ }
}
- request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
-
return 0;
}
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/7] drm/i915: Convert intel_lr_context_pin() for requests
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
` (2 preceding siblings ...)
2015-07-03 14:09 ` [PATCH 3/7] drm/i915: Assign request ringbuf before pin Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-03 14:09 ` [PATCH 5/7] drm/i915: Convert execlists_elsp_writ() " Mika Kuoppala
` (3 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Pass around requests to carry context deeper in callchain.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 8 +++-----
drivers/gpu/drm/i915/intel_lrc.c | 31 +++++++++++++++----------------
drivers/gpu/drm/i915/intel_lrc.h | 3 +--
3 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 49016e0..927964b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2620,10 +2620,8 @@ void i915_gem_request_free(struct kref *req_ref)
if (ctx) {
if (i915.enable_execlists) {
- struct intel_engine_cs *ring = req->ring;
-
- if (ctx != ring->default_context)
- intel_lr_context_unpin(ring, ctx);
+ if (ctx != req->ring->default_context)
+ intel_lr_context_unpin(req);
}
i915_gem_context_unreference(ctx);
@@ -2765,7 +2763,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
list_del(&submit_req->execlist_link);
if (submit_req->ctx != ring->default_context)
- intel_lr_context_unpin(ring, submit_req->ctx);
+ intel_lr_context_unpin(submit_req);
i915_gem_request_unreference(submit_req);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ab8a98c..36f8660 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -211,8 +211,7 @@ enum {
#define GEN8_CTX_ID_SHIFT 32
#define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17
-static int intel_lr_context_pin(struct intel_engine_cs *ring,
- struct intel_context *ctx);
+static int intel_lr_context_pin(struct drm_i915_gem_request *rq);
/**
* intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
@@ -541,7 +540,7 @@ static int execlists_context_queue(struct drm_i915_gem_request *request)
int num_elements = 0;
if (request->ctx != ring->default_context)
- intel_lr_context_pin(ring, request->ctx);
+ intel_lr_context_pin(request);
i915_gem_request_reference(request);
@@ -636,7 +635,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
if (request->ctx != request->ring->default_context) {
- ret = intel_lr_context_pin(request->ring, request->ctx);
+ ret = intel_lr_context_pin(request);
if (ret) {
request->ringbuf = NULL;
return ret;
@@ -952,7 +951,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
ctx->engine[ring->id].state;
if (ctx_obj && (ctx != ring->default_context))
- intel_lr_context_unpin(ring, ctx);
+ intel_lr_context_unpin(req);
list_del(&req->execlist_link);
i915_gem_request_unreference(req);
}
@@ -996,15 +995,15 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
return 0;
}
-static int intel_lr_context_pin(struct intel_engine_cs *ring,
- struct intel_context *ctx)
+static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
{
- struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+ struct intel_engine_cs *ring = rq->ring;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+ struct intel_ringbuffer *ringbuf = rq->ringbuf;
int ret = 0;
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
- if (ctx->engine[ring->id].pin_count++ == 0) {
+ if (rq->ctx->engine[ring->id].pin_count++ == 0) {
ret = i915_gem_obj_ggtt_pin(ctx_obj,
GEN8_LR_CONTEXT_ALIGN, 0);
if (ret)
@@ -1020,20 +1019,20 @@ static int intel_lr_context_pin(struct intel_engine_cs *ring,
unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
reset_pin_count:
- ctx->engine[ring->id].pin_count = 0;
+ rq->ctx->engine[ring->id].pin_count = 0;
return ret;
}
-void intel_lr_context_unpin(struct intel_engine_cs *ring,
- struct intel_context *ctx)
+void intel_lr_context_unpin(struct drm_i915_gem_request *rq)
{
- struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+ struct intel_engine_cs *ring = rq->ring;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+ struct intel_ringbuffer *ringbuf = rq->ringbuf;
if (ctx_obj) {
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
- if (--ctx->engine[ring->id].pin_count == 0) {
+ if (--rq->ctx->engine[ring->id].pin_count == 0) {
intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index f59940a..5c2baf2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -69,8 +69,7 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
void intel_lr_context_free(struct intel_context *ctx);
int intel_lr_context_deferred_create(struct intel_context *ctx,
struct intel_engine_cs *ring);
-void intel_lr_context_unpin(struct intel_engine_cs *ring,
- struct intel_context *ctx);
+void intel_lr_context_unpin(struct drm_i915_gem_request *req);
void intel_lr_context_reset(struct drm_device *dev,
struct intel_context *ctx);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/7] drm/i915: Convert execlists_elsp_writ() for requests
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
` (3 preceding siblings ...)
2015-07-03 14:09 ` [PATCH 4/7] drm/i915: Convert intel_lr_context_pin() for requests Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-03 14:09 ` [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() " Mika Kuoppala
` (2 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Pass around requests to carry context deeper in callchain.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 36f8660..c3c029a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -292,12 +292,16 @@ static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
return desc;
}
-static void execlists_elsp_write(struct intel_engine_cs *ring,
- struct drm_i915_gem_object *ctx_obj0,
- struct drm_i915_gem_object *ctx_obj1)
+static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
+ struct drm_i915_gem_request *rq1)
{
+
+ struct intel_engine_cs *ring = rq0->ring;
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
+ struct drm_i915_gem_object *ctx_obj1 = rq1 ?
+ rq1->ctx->engine[ring->id].state : NULL;
uint64_t temp = 0;
uint32_t desc[4];
@@ -365,18 +369,12 @@ static int execlists_update_context(struct drm_i915_gem_request *rq)
static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
struct drm_i915_gem_request *rq1)
{
- struct intel_engine_cs *ring = rq0->ring;
- struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
- struct drm_i915_gem_object *ctx_obj1 = NULL;
-
execlists_update_context(rq0);
- if (rq1) {
+ if (rq1)
execlists_update_context(rq1);
- ctx_obj1 = rq1->ctx->engine[ring->id].state;
- }
- execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
+ execlists_elsp_write(rq0, rq1);
}
static void execlists_context_unqueue(struct intel_engine_cs *ring)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() for requests
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
` (4 preceding siblings ...)
2015-07-03 14:09 ` [PATCH 5/7] drm/i915: Convert execlists_elsp_writ() " Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-08 16:40 ` Yu Dai
2015-07-03 14:09 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw Mika Kuoppala
2015-07-03 15:38 ` [RFC 0/7] Convert parts of intel_lrc.c to requests Chris Wilson
7 siblings, 1 reply; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Pass around requests to carry context deeper in callchain.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c3c029a..67ff460 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -261,10 +261,11 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
return lrca >> 12;
}
-static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
- struct drm_i915_gem_object *ctx_obj)
+static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request *rq)
{
+ struct intel_engine_cs *ring = rq->ring;
struct drm_device *dev = ring->dev;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
uint64_t desc;
uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj);
@@ -299,21 +300,18 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
struct intel_engine_cs *ring = rq0->ring;
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
- struct drm_i915_gem_object *ctx_obj1 = rq1 ?
- rq1->ctx->engine[ring->id].state : NULL;
uint64_t temp = 0;
uint32_t desc[4];
/* XXX: You must always write both descriptors in the order below. */
- if (ctx_obj1)
- temp = execlists_ctx_descriptor(ring, ctx_obj1);
+ if (rq1)
+ temp = execlists_ctx_descriptor(rq1);
else
temp = 0;
desc[1] = (u32)(temp >> 32);
desc[0] = (u32)temp;
- temp = execlists_ctx_descriptor(ring, ctx_obj0);
+ temp = execlists_ctx_descriptor(rq0);
desc[3] = (u32)(temp >> 32);
desc[2] = (u32)temp;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
` (5 preceding siblings ...)
2015-07-03 14:09 ` [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() " Mika Kuoppala
@ 2015-07-03 14:09 ` Mika Kuoppala
2015-07-03 15:36 ` Chris Wilson
2015-07-05 3:34 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw shuang.he
2015-07-03 15:38 ` [RFC 0/7] Convert parts of intel_lrc.c to requests Chris Wilson
7 siblings, 2 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-03 14:09 UTC (permalink / raw
To: intel-gfx; +Cc: miku
Now when we have requests this deep on call chain, we
can mark the elsp being submitted when it actually is.
While we are it, remove unnecessary temp assignment as
it is already initialized as zero.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 67ff460..8bf4acb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -304,14 +304,16 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
uint32_t desc[4];
/* XXX: You must always write both descriptors in the order below. */
- if (rq1)
+ if (rq1) {
temp = execlists_ctx_descriptor(rq1);
- else
- temp = 0;
+ rq1->elsp_submitted++;
+ }
+
desc[1] = (u32)(temp >> 32);
desc[0] = (u32)temp;
temp = execlists_ctx_descriptor(rq0);
+ rq0->elsp_submitted++;
desc[3] = (u32)(temp >> 32);
desc[2] = (u32)temp;
@@ -433,10 +435,6 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
WARN_ON(req1 && req1->elsp_submitted);
execlists_submit_requests(req0, req1);
-
- req0->elsp_submitted++;
- if (req1)
- req1->elsp_submitted++;
}
static bool execlists_check_remove_request(struct intel_engine_cs *ring,
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] drm/i915: Assign request ringbuf before pin
2015-07-03 14:09 ` [PATCH 3/7] drm/i915: Assign request ringbuf before pin Mika Kuoppala
@ 2015-07-03 14:11 ` Chris Wilson
2015-07-06 8:08 ` Mika Kuoppala
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2015-07-03 14:11 UTC (permalink / raw
To: Mika Kuoppala; +Cc: intel-gfx, miku
On Fri, Jul 03, 2015 at 05:09:34PM +0300, Mika Kuoppala wrote:
> In preparation to make intel_lr_context_pin|unpin to accept
> requests, assign ringbuf into request before we call the pinning.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4139eb6..ab8a98c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -633,14 +633,16 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
> {
> int ret;
>
> + request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
> +
> if (request->ctx != request->ring->default_context) {
> ret = intel_lr_context_pin(request->ring, request->ctx);
> - if (ret)
> + if (ret) {
> + request->ringbuf = NULL;
You don't need to unset it again. We just trash the request on error.
-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] 24+ messages in thread
* Re: [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-03 14:09 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw Mika Kuoppala
@ 2015-07-03 15:36 ` Chris Wilson
2015-07-06 8:09 ` Mika Kuoppala
2015-07-05 3:34 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw shuang.he
1 sibling, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2015-07-03 15:36 UTC (permalink / raw
To: Mika Kuoppala; +Cc: intel-gfx, miku
On Fri, Jul 03, 2015 at 05:09:38PM +0300, Mika Kuoppala wrote:
> Now when we have requests this deep on call chain, we
> can mark the elsp being submitted when it actually is.
> While we are it, remove unnecessary temp assignment as
> it is already initialized as zero.
Bah, which I think is bad practice (because when looking at patch contexts
like this, you have no idea if that is true or not as you can't see the
value). You could reduce the number of writes if you wanted to. Personally
I went with
uint32_t desc[4];
if (ring->execlist_port[1]) {
desc[0] = execlists_request_write_tail(ring,
ring->execlist_port[1]);
desc[1] = ring->execlist_port[1]->seqno;
} else
desc[1] = desc[0] = 0;
desc[2] = execlists_request_write_tail(ring, ring->execlist_port[0]);
desc[3] = ring->execlist_port[0]->seqno;
-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] 24+ messages in thread
* Re: [RFC 0/7] Convert parts of intel_lrc.c to requests
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
` (6 preceding siblings ...)
2015-07-03 14:09 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw Mika Kuoppala
@ 2015-07-03 15:38 ` Chris Wilson
7 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2015-07-03 15:38 UTC (permalink / raw
To: Mika Kuoppala; +Cc: intel-gfx, miku
On Fri, Jul 03, 2015 at 05:09:31PM +0300, Mika Kuoppala wrote:
> Hi,
>
> Reader might think that I did this for only mark few elsps
> done deeper in callchain. But the reason is that I wanted
> to test more uniqueness guarantees for generation of context ids
> that is passed for hardware. And found out that the current
> code structure strips out all the request/context very early on.
It's a start. Most of the patches could be merged since they seem to be
interrelated and not doing anything startling.
-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] 24+ messages in thread
* Re: [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-03 14:09 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw Mika Kuoppala
2015-07-03 15:36 ` Chris Wilson
@ 2015-07-05 3:34 ` shuang.he
1 sibling, 0 replies; 24+ messages in thread
From: shuang.he @ 2015-07-05 3:34 UTC (permalink / raw
To: shuang.he, lei.a.liu, intel-gfx, mika.kuoppala
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6720
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 312/316 312/316
IVB 343/343 343/343
BYT -1 287/287 286/287
HSW 380/380 380/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/7] drm/i915: Assign request ringbuf before pin
2015-07-03 14:11 ` Chris Wilson
@ 2015-07-06 8:08 ` Mika Kuoppala
0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-06 8:08 UTC (permalink / raw
To: intel-gfx
In preparation to make intel_lr_context_pin|unpin to accept
requests, assign ringbuf into request before we call the pinning.
v2: No need to unset ringbuf on error path (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4139eb6..ace3a98 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -633,14 +633,14 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
{
int ret;
+ request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
+
if (request->ctx != request->ring->default_context) {
ret = intel_lr_context_pin(request->ring, request->ctx);
if (ret)
return ret;
}
- request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
-
return 0;
}
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-03 15:36 ` Chris Wilson
@ 2015-07-06 8:09 ` Mika Kuoppala
2015-07-06 9:25 ` Chris Wilson
0 siblings, 1 reply; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-06 8:09 UTC (permalink / raw
To: intel-gfx
Now when we have requests this deep on call chain, we can mark
the elsp being submitted when it actually is. Remove temp variable
and readjust commenting to more closely fit to the code.
v2: Avoid tmp variable and reduce number of writes (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 36 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 468f569..de35db6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -300,31 +300,29 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
struct intel_engine_cs *ring = rq0->ring;
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- uint64_t temp = 0;
- uint32_t desc[4];
+ uint64_t desc[2];
- /* XXX: You must always write both descriptors in the order below. */
- if (rq1)
- temp = execlists_ctx_descriptor(rq1);
- else
- temp = 0;
- desc[1] = (u32)(temp >> 32);
- desc[0] = (u32)temp;
+ if (rq1) {
+ desc[1] = execlists_ctx_descriptor(rq1);
+ rq1->elsp_submitted++;
+ } else {
+ desc[1] = 0;
+ }
- temp = execlists_ctx_descriptor(rq0);
- desc[3] = (u32)(temp >> 32);
- desc[2] = (u32)temp;
+ desc[0] = execlists_ctx_descriptor(rq0);
+ rq0->elsp_submitted++;
+ /* You must always write both descriptors in the order below. */
spin_lock(&dev_priv->uncore.lock);
intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
- I915_WRITE_FW(RING_ELSP(ring), desc[1]);
- I915_WRITE_FW(RING_ELSP(ring), desc[0]);
- I915_WRITE_FW(RING_ELSP(ring), desc[3]);
+ I915_WRITE_FW(RING_ELSP(ring), upper_32_bits(desc[1]));
+ I915_WRITE_FW(RING_ELSP(ring), lower_32_bits(desc[1]));
+ I915_WRITE_FW(RING_ELSP(ring), upper_32_bits(desc[0]));
/* The context is automatically loaded after the following */
- I915_WRITE_FW(RING_ELSP(ring), desc[2]);
+ I915_WRITE_FW(RING_ELSP(ring), lower_32_bits(desc[0]));
- /* ELSP is a wo register, so use another nearby reg for posting instead */
+ /* ELSP is a wo register, use another nearby reg for posting */
POSTING_READ_FW(RING_EXECLIST_STATUS(ring));
intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
spin_unlock(&dev_priv->uncore.lock);
@@ -433,10 +431,6 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
WARN_ON(req1 && req1->elsp_submitted);
execlists_submit_requests(req0, req1);
-
- req0->elsp_submitted++;
- if (req1)
- req1->elsp_submitted++;
}
static bool execlists_check_remove_request(struct intel_engine_cs *ring,
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-06 8:09 ` Mika Kuoppala
@ 2015-07-06 9:25 ` Chris Wilson
2015-07-06 9:32 ` Chris Wilson
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2015-07-06 9:25 UTC (permalink / raw
To: Mika Kuoppala; +Cc: intel-gfx
On Mon, Jul 06, 2015 at 11:09:25AM +0300, Mika Kuoppala wrote:
> Now when we have requests this deep on call chain, we can mark
> the elsp being submitted when it actually is. Remove temp variable
> and readjust commenting to more closely fit to the code.
>
> v2: Avoid tmp variable and reduce number of writes (Chris)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Looks cleaner to me,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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] 24+ messages in thread
* Re: [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-06 9:25 ` Chris Wilson
@ 2015-07-06 9:32 ` Chris Wilson
2015-07-06 14:52 ` Daniel Vetter
2015-07-06 15:35 ` [PATCH 1/2] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
0 siblings, 2 replies; 24+ messages in thread
From: Chris Wilson @ 2015-07-06 9:32 UTC (permalink / raw
To: Mika Kuoppala, intel-gfx
On Mon, Jul 06, 2015 at 10:25:17AM +0100, Chris Wilson wrote:
> On Mon, Jul 06, 2015 at 11:09:25AM +0300, Mika Kuoppala wrote:
> > Now when we have requests this deep on call chain, we can mark
> > the elsp being submitted when it actually is. Remove temp variable
> > and readjust commenting to more closely fit to the code.
> >
> > v2: Avoid tmp variable and reduce number of writes (Chris)
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
> Looks cleaner to me,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
In fact, nothing in the series scared me and with the two minor changes,
might as well apply that r-b to the whole series. For the benefit of
others, can you repost the contracted series with my r-b applied?
-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] 24+ messages in thread
* Re: [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw
2015-07-06 9:32 ` Chris Wilson
@ 2015-07-06 14:52 ` Daniel Vetter
2015-07-06 15:35 ` [PATCH 1/2] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
1 sibling, 0 replies; 24+ messages in thread
From: Daniel Vetter @ 2015-07-06 14:52 UTC (permalink / raw
To: Chris Wilson, Mika Kuoppala, intel-gfx
On Mon, Jul 06, 2015 at 10:32:11AM +0100, Chris Wilson wrote:
> On Mon, Jul 06, 2015 at 10:25:17AM +0100, Chris Wilson wrote:
> > On Mon, Jul 06, 2015 at 11:09:25AM +0300, Mika Kuoppala wrote:
> > > Now when we have requests this deep on call chain, we can mark
> > > the elsp being submitted when it actually is. Remove temp variable
> > > and readjust commenting to more closely fit to the code.
> > >
> > > v2: Avoid tmp variable and reduce number of writes (Chris)
> > >
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >
> > Looks cleaner to me,
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> In fact, nothing in the series scared me and with the two minor changes,
> might as well apply that r-b to the whole series. For the benefit of
> others, can you repost the contracted series with my r-b applied?
Yeah maybe this was a bit too much splitting but I didn't find it too much
so just went ahead and applied it all.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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] 24+ messages in thread
* [PATCH 1/2] drm/i915: Convert execlist_submit_contexts() for requests
2015-07-06 9:32 ` Chris Wilson
2015-07-06 14:52 ` Daniel Vetter
@ 2015-07-06 15:35 ` Mika Kuoppala
1 sibling, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-06 15:35 UTC (permalink / raw
To: intel-gfx
Make execlist_submit_context() accept requests instead of engine
context objects, also renaming it to execlist_submit_requests()
to better reflect the functionality. Keep passing requests as
first class objects, all the way down to where elsps are written.
The benefits are that with requests, we can mold the request state
where the actual hardware interactions happen. Author also expects
more gains in debuggability and more leverage on context descriptor
creation.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 8 ++-
drivers/gpu/drm/i915/intel_lrc.c | 103 +++++++++++++++++----------------------
drivers/gpu/drm/i915/intel_lrc.h | 3 +-
3 files changed, 49 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 49016e0..927964b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2620,10 +2620,8 @@ void i915_gem_request_free(struct kref *req_ref)
if (ctx) {
if (i915.enable_execlists) {
- struct intel_engine_cs *ring = req->ring;
-
- if (ctx != ring->default_context)
- intel_lr_context_unpin(ring, ctx);
+ if (ctx != req->ring->default_context)
+ intel_lr_context_unpin(req);
}
i915_gem_context_unreference(ctx);
@@ -2765,7 +2763,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
list_del(&submit_req->execlist_link);
if (submit_req->ctx != ring->default_context)
- intel_lr_context_unpin(ring, submit_req->ctx);
+ intel_lr_context_unpin(submit_req);
i915_gem_request_unreference(submit_req);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5534f87..74601fa 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -211,8 +211,7 @@ enum {
#define GEN8_CTX_ID_SHIFT 32
#define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17
-static int intel_lr_context_pin(struct intel_engine_cs *ring,
- struct intel_context *ctx);
+static int intel_lr_context_pin(struct drm_i915_gem_request *rq);
/**
* intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
@@ -262,10 +261,11 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
return lrca >> 12;
}
-static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
- struct drm_i915_gem_object *ctx_obj)
+static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request *rq)
{
+ struct intel_engine_cs *ring = rq->ring;
struct drm_device *dev = ring->dev;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
uint64_t desc;
uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj);
@@ -293,24 +293,25 @@ static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
return desc;
}
-static void execlists_elsp_write(struct intel_engine_cs *ring,
- struct drm_i915_gem_object *ctx_obj0,
- struct drm_i915_gem_object *ctx_obj1)
+static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
+ struct drm_i915_gem_request *rq1)
{
+
+ struct intel_engine_cs *ring = rq0->ring;
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
uint64_t temp = 0;
uint32_t desc[4];
/* XXX: You must always write both descriptors in the order below. */
- if (ctx_obj1)
- temp = execlists_ctx_descriptor(ring, ctx_obj1);
+ if (rq1)
+ temp = execlists_ctx_descriptor(rq1);
else
temp = 0;
desc[1] = (u32)(temp >> 32);
desc[0] = (u32)temp;
- temp = execlists_ctx_descriptor(ring, ctx_obj0);
+ temp = execlists_ctx_descriptor(rq0);
desc[3] = (u32)(temp >> 32);
desc[2] = (u32)temp;
@@ -329,19 +330,24 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
spin_unlock(&dev_priv->uncore.lock);
}
-static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
- struct drm_i915_gem_object *ring_obj,
- struct i915_hw_ppgtt *ppgtt,
- u32 tail)
+static int execlists_update_context(struct drm_i915_gem_request *rq)
{
+ struct intel_engine_cs *ring = rq->ring;
+ struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+ struct drm_i915_gem_object *rb_obj = rq->ringbuf->obj;
struct page *page;
uint32_t *reg_state;
+ BUG_ON(!ctx_obj);
+ WARN_ON(!i915_gem_obj_is_pinned(ctx_obj));
+ WARN_ON(!i915_gem_obj_is_pinned(rb_obj));
+
page = i915_gem_object_get_page(ctx_obj, 1);
reg_state = kmap_atomic(page);
- reg_state[CTX_RING_TAIL+1] = tail;
- reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(ring_obj);
+ reg_state[CTX_RING_TAIL+1] = rq->tail;
+ reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(rb_obj);
/* True PPGTT with dynamic page allocation: update PDP registers and
* point the unallocated PDPs to the scratch page
@@ -358,32 +364,15 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
return 0;
}
-static void execlists_submit_contexts(struct intel_engine_cs *ring,
- struct intel_context *to0, u32 tail0,
- struct intel_context *to1, u32 tail1)
+static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
+ struct drm_i915_gem_request *rq1)
{
- struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
- struct drm_i915_gem_object *ctx_obj1 = NULL;
- struct intel_ringbuffer *ringbuf1 = NULL;
-
- BUG_ON(!ctx_obj0);
- WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
- WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
-
- execlists_update_context(ctx_obj0, ringbuf0->obj, to0->ppgtt, tail0);
-
- if (to1) {
- ringbuf1 = to1->engine[ring->id].ringbuf;
- ctx_obj1 = to1->engine[ring->id].state;
- BUG_ON(!ctx_obj1);
- WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
- WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
+ execlists_update_context(rq0);
- execlists_update_context(ctx_obj1, ringbuf1->obj, to1->ppgtt, tail1);
- }
+ if (rq1)
+ execlists_update_context(rq1);
- execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
+ execlists_elsp_write(rq0, rq1);
}
static void execlists_context_unqueue(struct intel_engine_cs *ring)
@@ -443,9 +432,7 @@ 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_requests(req0, req1);
req0->elsp_submitted++;
if (req1)
@@ -549,7 +536,7 @@ static int execlists_context_queue(struct drm_i915_gem_request *request)
int num_elements = 0;
if (request->ctx != ring->default_context)
- intel_lr_context_pin(ring, request->ctx);
+ intel_lr_context_pin(request);
i915_gem_request_reference(request);
@@ -641,14 +628,14 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
{
int ret;
+ request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
+
if (request->ctx != request->ring->default_context) {
- ret = intel_lr_context_pin(request->ring, request->ctx);
+ ret = intel_lr_context_pin(request);
if (ret)
return ret;
}
- request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
-
return 0;
}
@@ -958,7 +945,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring)
ctx->engine[ring->id].state;
if (ctx_obj && (ctx != ring->default_context))
- intel_lr_context_unpin(ring, ctx);
+ intel_lr_context_unpin(req);
list_del(&req->execlist_link);
i915_gem_request_unreference(req);
}
@@ -1002,15 +989,15 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req)
return 0;
}
-static int intel_lr_context_pin(struct intel_engine_cs *ring,
- struct intel_context *ctx)
+static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
{
- struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+ struct intel_engine_cs *ring = rq->ring;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+ struct intel_ringbuffer *ringbuf = rq->ringbuf;
int ret = 0;
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
- if (ctx->engine[ring->id].pin_count++ == 0) {
+ if (rq->ctx->engine[ring->id].pin_count++ == 0) {
ret = i915_gem_obj_ggtt_pin(ctx_obj,
GEN8_LR_CONTEXT_ALIGN, 0);
if (ret)
@@ -1026,20 +1013,20 @@ static int intel_lr_context_pin(struct intel_engine_cs *ring,
unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
reset_pin_count:
- ctx->engine[ring->id].pin_count = 0;
+ rq->ctx->engine[ring->id].pin_count = 0;
return ret;
}
-void intel_lr_context_unpin(struct intel_engine_cs *ring,
- struct intel_context *ctx)
+void intel_lr_context_unpin(struct drm_i915_gem_request *rq)
{
- struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+ struct intel_engine_cs *ring = rq->ring;
+ struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
+ struct intel_ringbuffer *ringbuf = rq->ringbuf;
if (ctx_obj) {
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
- if (--ctx->engine[ring->id].pin_count == 0) {
+ if (--rq->ctx->engine[ring->id].pin_count == 0) {
intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index d3dd3ac..e0299fb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -70,8 +70,7 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
void intel_lr_context_free(struct intel_context *ctx);
int intel_lr_context_deferred_create(struct intel_context *ctx,
struct intel_engine_cs *ring);
-void intel_lr_context_unpin(struct intel_engine_cs *ring,
- struct intel_context *ctx);
+void intel_lr_context_unpin(struct drm_i915_gem_request *req);
void intel_lr_context_reset(struct drm_device *dev,
struct intel_context *ctx);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests
2015-07-03 14:09 ` [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
@ 2015-07-07 16:49 ` Yu Dai
2015-07-07 21:44 ` Yu Dai
0 siblings, 1 reply; 24+ messages in thread
From: Yu Dai @ 2015-07-07 16:49 UTC (permalink / raw
To: intel-gfx
On 07/03/2015 07:09 AM, Mika Kuoppala wrote:
> Pass around requests to carry context deeper in callchain.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 9b928ab..583210d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -358,29 +358,29 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
> return 0;
> }
>
> -static void execlists_submit_contexts(struct intel_engine_cs *ring,
> - struct intel_context *to0, u32 tail0,
> - struct intel_context *to1, u32 tail1)
> +static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
> + struct drm_i915_gem_request *rq1)
> {
> - struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
> - struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
> + struct intel_engine_cs *ring = rq0->ring;
> + struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
> struct drm_i915_gem_object *ctx_obj1 = NULL;
> - struct intel_ringbuffer *ringbuf1 = NULL;
>
> BUG_ON(!ctx_obj0);
> WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
> - WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
> + WARN_ON(!i915_gem_obj_is_pinned(rq0->ringbuf->obj));
>
> - execlists_update_context(ctx_obj0, ringbuf0->obj, to0->ppgtt, tail0);
> + execlists_update_context(ctx_obj1, rq0->ringbuf->obj,
Typo? Here it should be ctx_obj0.
> + rq0->ctx->ppgtt, rq0->tail);
> +
> + if (rq1) {
> + ctx_obj1 = rq1->ctx->engine[ring->id].state;
>
> - if (to1) {
> - ringbuf1 = to1->engine[ring->id].ringbuf;
> - ctx_obj1 = to1->engine[ring->id].state;
> BUG_ON(!ctx_obj1);
> WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
> - WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
> + WARN_ON(!i915_gem_obj_is_pinned(rq1->ringbuf->obj));
>
> - execlists_update_context(ctx_obj1, ringbuf1->obj, to1->ppgtt, tail1);
> + execlists_update_context(ctx_obj1, rq1->ringbuf->obj,
> + rq1->ctx->ppgtt, rq1->tail);
> }
>
> execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
> @@ -443,9 +443,7 @@ 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_requests(req0, req1);
>
> req0->elsp_submitted++;
> if (req1)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests
2015-07-07 16:49 ` Yu Dai
@ 2015-07-07 21:44 ` Yu Dai
2015-07-08 7:23 ` Mika Kuoppala
0 siblings, 1 reply; 24+ messages in thread
From: Yu Dai @ 2015-07-07 21:44 UTC (permalink / raw
To: intel-gfx
On 07/07/2015 09:49 AM, Yu Dai wrote:
> On 07/03/2015 07:09 AM, Mika Kuoppala wrote:
> > Pass around requests to carry context deeper in callchain.
> >
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_lrc.c | 30 ++++++++++++++----------------
> > 1 file changed, 14 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 9b928ab..583210d 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -358,29 +358,29 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
> > return 0;
> > }
> >
> > -static void execlists_submit_contexts(struct intel_engine_cs *ring,
> > - struct intel_context *to0, u32 tail0,
> > - struct intel_context *to1, u32 tail1)
> > +static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
> > + struct drm_i915_gem_request *rq1)
> > {
> > - struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
> > - struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
> > + struct intel_engine_cs *ring = rq0->ring;
> > + struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
> > struct drm_i915_gem_object *ctx_obj1 = NULL;
> > - struct intel_ringbuffer *ringbuf1 = NULL;
> >
> > BUG_ON(!ctx_obj0);
> > WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
> > - WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
> > + WARN_ON(!i915_gem_obj_is_pinned(rq0->ringbuf->obj));
> >
> > - execlists_update_context(ctx_obj0, ringbuf0->obj, to0->ppgtt, tail0);
> > + execlists_update_context(ctx_obj1, rq0->ringbuf->obj,
> Typo? Here it should be ctx_obj0.
Never mind. Issue got fixed by patch 2/7.
Alex
> > + rq0->ctx->ppgtt, rq0->tail);
> > +
> > + if (rq1) {
> > + ctx_obj1 = rq1->ctx->engine[ring->id].state;
> >
> > - if (to1) {
> > - ringbuf1 = to1->engine[ring->id].ringbuf;
> > - ctx_obj1 = to1->engine[ring->id].state;
> > BUG_ON(!ctx_obj1);
> > WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
> > - WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
> > + WARN_ON(!i915_gem_obj_is_pinned(rq1->ringbuf->obj));
> >
> > - execlists_update_context(ctx_obj1, ringbuf1->obj, to1->ppgtt, tail1);
> > + execlists_update_context(ctx_obj1, rq1->ringbuf->obj,
> > + rq1->ctx->ppgtt, rq1->tail);
> > }
> >
> > execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
> > @@ -443,9 +443,7 @@ 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_requests(req0, req1);
> >
> > req0->elsp_submitted++;
> > if (req1)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests
2015-07-07 21:44 ` Yu Dai
@ 2015-07-08 7:23 ` Mika Kuoppala
0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-08 7:23 UTC (permalink / raw
To: Yu Dai, intel-gfx
Yu Dai <yu.dai@intel.com> writes:
> On 07/07/2015 09:49 AM, Yu Dai wrote:
>> On 07/03/2015 07:09 AM, Mika Kuoppala wrote:
>> > Pass around requests to carry context deeper in callchain.
>> >
>> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_lrc.c | 30 ++++++++++++++----------------
>> > 1 file changed, 14 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> > index 9b928ab..583210d 100644
>> > --- a/drivers/gpu/drm/i915/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> > @@ -358,29 +358,29 @@ static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
>> > return 0;
>> > }
>> >
>> > -static void execlists_submit_contexts(struct intel_engine_cs *ring,
>> > - struct intel_context *to0, u32 tail0,
>> > - struct intel_context *to1, u32 tail1)
>> > +static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
>> > + struct drm_i915_gem_request *rq1)
>> > {
>> > - struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
>> > - struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
>> > + struct intel_engine_cs *ring = rq0->ring;
>> > + struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
>> > struct drm_i915_gem_object *ctx_obj1 = NULL;
>> > - struct intel_ringbuffer *ringbuf1 = NULL;
>> >
>> > BUG_ON(!ctx_obj0);
>> > WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
>> > - WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
>> > + WARN_ON(!i915_gem_obj_is_pinned(rq0->ringbuf->obj));
>> >
>> > - execlists_update_context(ctx_obj0, ringbuf0->obj, to0->ppgtt, tail0);
>> > + execlists_update_context(ctx_obj1, rq0->ringbuf->obj,
>> Typo? Here it should be ctx_obj0.
>
> Never mind. Issue got fixed by patch 2/7.
> Alex
Well that will cause trouble for the bisecter :(
-Mika
>> > + rq0->ctx->ppgtt, rq0->tail);
>> > +
>> > + if (rq1) {
>> > + ctx_obj1 = rq1->ctx->engine[ring->id].state;
>> >
>> > - if (to1) {
>> > - ringbuf1 = to1->engine[ring->id].ringbuf;
>> > - ctx_obj1 = to1->engine[ring->id].state;
>> > BUG_ON(!ctx_obj1);
>> > WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
>> > - WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
>> > + WARN_ON(!i915_gem_obj_is_pinned(rq1->ringbuf->obj));
>> >
>> > - execlists_update_context(ctx_obj1, ringbuf1->obj, to1->ppgtt, tail1);
>> > + execlists_update_context(ctx_obj1, rq1->ringbuf->obj,
>> > + rq1->ctx->ppgtt, rq1->tail);
>> > }
>> >
>> > execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
>> > @@ -443,9 +443,7 @@ 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_requests(req0, req1);
>> >
>> > req0->elsp_submitted++;
>> > if (req1)
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() for requests
2015-07-03 14:09 ` [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() " Mika Kuoppala
@ 2015-07-08 16:40 ` Yu Dai
2015-07-08 17:04 ` Dave Gordon
0 siblings, 1 reply; 24+ messages in thread
From: Yu Dai @ 2015-07-08 16:40 UTC (permalink / raw
To: Mika Kuoppala, intel-gfx; +Cc: miku
On 07/03/2015 07:09 AM, Mika Kuoppala wrote:
> Pass around requests to carry context deeper in callchain.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index c3c029a..67ff460 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -261,10 +261,11 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
> return lrca >> 12;
> }
>
> -static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
> - struct drm_i915_gem_object *ctx_obj)
> +static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request *rq)
> {
GuC patch series tries to utilize this function. However, changing from
ring/context to request makes it unusable in the case where request is
not needed (not available). This context descriptor has nothing to do
with drm_i915_gem_request IMO. And it is waste of time to call it for
each batch submission. This value is fixed when context is pinned. Maybe
add a member ctx_desc into intel_context->engine; initialize the value
within intel_lr_context_pin; then use it whenever needed.
Thanks,
Alex
> + struct intel_engine_cs *ring = rq->ring;
> struct drm_device *dev = ring->dev;
> + struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
> uint64_t desc;
> uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj);
>
> @@ -299,21 +300,18 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
> struct intel_engine_cs *ring = rq0->ring;
> struct drm_device *dev = ring->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct drm_i915_gem_object *ctx_obj0 = rq0->ctx->engine[ring->id].state;
> - struct drm_i915_gem_object *ctx_obj1 = rq1 ?
> - rq1->ctx->engine[ring->id].state : NULL;
> uint64_t temp = 0;
> uint32_t desc[4];
>
> /* XXX: You must always write both descriptors in the order below. */
> - if (ctx_obj1)
> - temp = execlists_ctx_descriptor(ring, ctx_obj1);
> + if (rq1)
> + temp = execlists_ctx_descriptor(rq1);
> else
> temp = 0;
> desc[1] = (u32)(temp >> 32);
> desc[0] = (u32)temp;
>
> - temp = execlists_ctx_descriptor(ring, ctx_obj0);
> + temp = execlists_ctx_descriptor(rq0);
> desc[3] = (u32)(temp >> 32);
> desc[2] = (u32)temp;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() for requests
2015-07-08 16:40 ` Yu Dai
@ 2015-07-08 17:04 ` Dave Gordon
2015-07-08 17:26 ` Mika Kuoppala
0 siblings, 1 reply; 24+ messages in thread
From: Dave Gordon @ 2015-07-08 17:04 UTC (permalink / raw
To: Yu Dai, Mika Kuoppala, intel-gfx; +Cc: miku
On 08/07/15 17:40, Yu Dai wrote:
> On 07/03/2015 07:09 AM, Mika Kuoppala wrote:
>> Pass around requests to carry context deeper in callchain.
>>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_lrc.c | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index c3c029a..67ff460 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -261,10 +261,11 @@ u32 intel_execlists_ctx_id(struct
>> drm_i915_gem_object *ctx_obj)
>> return lrca >> 12;
>> }
>> -static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
>> - struct drm_i915_gem_object *ctx_obj)
>> +static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request
>> *rq)
>> {
>
> GuC patch series tries to utilize this function. However, changing from
> ring/context to request makes it unusable in the case where request is
> not needed (not available). This context descriptor has nothing to do
> with drm_i915_gem_request IMO. And it is waste of time to call it for
> each batch submission. This value is fixed when context is pinned. Maybe
> add a member ctx_desc into intel_context->engine; initialize the value
> within intel_lr_context_pin; then use it whenever needed.
>
> Thanks,
> Alex
Yes, I've effectively reversed this in the next GuC submission series,
since we don't have a request during setup, and this function doesn't
actually need one either; it's only being used as a handle for
extracting the context and ring.
So in my version it will take an intel_context and a ring so it's up to
the caller to extract those from the drm_i915_gem_request (rq->ctx) and
(rq->ring) and pass them separately. Then the GuC can use it during
setup as well as at runtime :)
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() for requests
2015-07-08 17:04 ` Dave Gordon
@ 2015-07-08 17:26 ` Mika Kuoppala
0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2015-07-08 17:26 UTC (permalink / raw
To: Dave Gordon; +Cc: intel-gfx
On Wed Jul 08 2015 at 18:04:44 +0100, Dave Gordon wrote:
> On 08/07/15 17:40, Yu Dai wrote:
> >On 07/03/2015 07:09 AM, Mika Kuoppala wrote:
> >>Pass around requests to carry context deeper in callchain.
> >>
> >>Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >>---
> >> drivers/gpu/drm/i915/intel_lrc.c | 14 ++++++--------
> >> 1 file changed, 6 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> >>b/drivers/gpu/drm/i915/intel_lrc.c
> >>index c3c029a..67ff460 100644
> >>--- a/drivers/gpu/drm/i915/intel_lrc.c
> >>+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >>@@ -261,10 +261,11 @@ u32 intel_execlists_ctx_id(struct
> >>drm_i915_gem_object *ctx_obj)
> >> return lrca >> 12;
> >> }
> >>-static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring,
> >>- struct drm_i915_gem_object *ctx_obj)
> >>+static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request
> >>*rq)
> >> {
> >
> >GuC patch series tries to utilize this function. However, changing from
> >ring/context to request makes it unusable in the case where request is
> >not needed (not available). This context descriptor has nothing to do
> >with drm_i915_gem_request IMO. And it is waste of time to call it for
> >each batch submission. This value is fixed when context is pinned. Maybe
> >add a member ctx_desc into intel_context->engine; initialize the value
> >within intel_lr_context_pin; then use it whenever needed.
> >
> >Thanks,
> >Alex
>
> Yes, I've effectively reversed this in the next GuC submission
> series, since we don't have a request during setup, and this
> function doesn't actually need one either; it's only being used as a
> handle for extracting the context and ring.
>
> So in my version it will take an intel_context and a ring so it's up
> to the caller to extract those from the drm_i915_gem_request
> (rq->ctx) and (rq->ring) and pass them separately. Then the GuC can
> use it during setup as well as at runtime :)
>
I admit I went one step too far on this one in the series. The bspec says
the context ids created should be unique across all engines and all execlists.
I was concerned that our lrca only setup and resubmission for
pre-emption would break this rule. (and be the reason for skl hickups)
The bspec has a note about creating context id with ring/counter(seqno)/lrca
combination. Thus the request -> descriptor creation.
I didn't post that patch as it didn't cure the skl hang. But it made
debugging the context status queue easier as seqnos were part of
context_id.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2015-07-08 17:53 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-03 14:09 [RFC 0/7] Convert parts of intel_lrc.c to requests Mika Kuoppala
2015-07-03 14:09 ` [PATCH 1/7] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
2015-07-07 16:49 ` Yu Dai
2015-07-07 21:44 ` Yu Dai
2015-07-08 7:23 ` Mika Kuoppala
2015-07-03 14:09 ` [PATCH 2/7] drm/i915: Convert execlists_update_context() " Mika Kuoppala
2015-07-03 14:09 ` [PATCH 3/7] drm/i915: Assign request ringbuf before pin Mika Kuoppala
2015-07-03 14:11 ` Chris Wilson
2015-07-06 8:08 ` Mika Kuoppala
2015-07-03 14:09 ` [PATCH 4/7] drm/i915: Convert intel_lr_context_pin() for requests Mika Kuoppala
2015-07-03 14:09 ` [PATCH 5/7] drm/i915: Convert execlists_elsp_writ() " Mika Kuoppala
2015-07-03 14:09 ` [PATCH 6/7] drm/i915: Convert execlists_ctx_descriptor() " Mika Kuoppala
2015-07-08 16:40 ` Yu Dai
2015-07-08 17:04 ` Dave Gordon
2015-07-08 17:26 ` Mika Kuoppala
2015-07-03 14:09 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw Mika Kuoppala
2015-07-03 15:36 ` Chris Wilson
2015-07-06 8:09 ` Mika Kuoppala
2015-07-06 9:25 ` Chris Wilson
2015-07-06 9:32 ` Chris Wilson
2015-07-06 14:52 ` Daniel Vetter
2015-07-06 15:35 ` [PATCH 1/2] drm/i915: Convert execlist_submit_contexts() for requests Mika Kuoppala
2015-07-05 3:34 ` [PATCH 7/7] drm/i915: Mark elsps submitted when they are pushed to hw shuang.he
2015-07-03 15:38 ` [RFC 0/7] Convert parts of intel_lrc.c to requests Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).