* [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
* 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
* [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
* 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
* [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 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
* 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
* [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 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
* [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 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
* 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
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