| Message ID | 1423828140-10653-4-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 13, 2015 at 11:48:12AM +0000, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > In execlist mode, the ringbuf is a function of the ring and context whereas in > legacy mode, it is derived from the ring alone. Thus the calculation required to > determine the ringbuf pointer from the ring (and context) also needs to test > execlist mode or not. This is messy. > > Further, the request structure holds a pointer to both the ring and the context > for which it was created. Thus, given a request, it is possible to derive the > ringbuf in either legacy or execlist mode. Hence it is necessary to pass just > the request in to all the low level functions rather than some combination of > request, ring, context and ringbuf. However, rather than recalculating it each > time, it is much simpler to just cache the ringbuf pointer in the request > structure itself. > > Caching the pointer means the calculation is done one at request creation time s/one/once/ I guess? > and all further code and simply read it directly from the request structure. > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > drivers/gpu/drm/i915/i915_gem.c | 14 +------------- > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f2a825e..e90b786 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2137,8 +2137,9 @@ struct drm_i915_gem_request { > /** Position in the ringbuffer of the end of the whole request */ > u32 tail; > > - /** Context related to this request */ > + /** Context and ring buffer related to this request */ > struct intel_context *ctx; > + struct intel_ringbuffer *ringbuf; > > /** Batch buffer related to this request if any */ > struct drm_i915_gem_object *batch_obj; > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c26d36c..2ebe914 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2758,7 +2758,6 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) > > while (!list_empty(&ring->request_list)) { > struct drm_i915_gem_request *request; > - struct intel_ringbuffer *ringbuf; > > request = list_first_entry(&ring->request_list, > struct drm_i915_gem_request, > @@ -2769,23 +2768,12 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) > > trace_i915_gem_request_retire(request); > > - /* This is one of the few common intersection points > - * between legacy ringbuffer submission and execlists: > - * we need to tell them apart in order to find the correct > - * ringbuffer to which the request belongs to. > - */ > - if (i915.enable_execlists) { > - struct intel_context *ctx = request->ctx; > - ringbuf = ctx->engine[ring->id].ringbuf; > - } else > - ringbuf = ring->buffer; > - > /* We know the GPU must have read the request to have > * sent us the seqno + interrupt, so use the position > * of tail of the request to update the last known position > * of the GPU head. > */ > - ringbuf->last_retired_head = request->postfix; > + request->ringbuf->last_retired_head = request->postfix; > > i915_gem_free_request(request); > } > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 73c1861..762136b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -878,12 +878,14 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring, > return ret; > } > > - /* Hold a reference to the context this request belongs to > + /* > + * Hold a reference to the context this request belongs to > * (we will need it when the time comes to emit/retire the > - * request). > + * request). Likewise, the ringbuff is useful to keep track of. > */ Imo this comment is a bit too obvious so I just deleted it ;-) -Daniel > request->ctx = ctx; > i915_gem_context_reference(request->ctx); > + request->ringbuf = ctx->engine[ring->id].ringbuf; > > ring->outstanding_lazy_request = request; > return 0; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index d611608..ca9e7e6 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2100,6 +2100,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) > > kref_init(&request->ref); > request->ring = ring; > + request->ringbuf = ring->buffer; > request->uniq = dev_private->request_uniq++; > > ret = i915_gem_get_seqno(ring->dev, &request->seqno); > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f2a825e..e90b786 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2137,8 +2137,9 @@ struct drm_i915_gem_request { /** Position in the ringbuffer of the end of the whole request */ u32 tail; - /** Context related to this request */ + /** Context and ring buffer related to this request */ struct intel_context *ctx; + struct intel_ringbuffer *ringbuf; /** Batch buffer related to this request if any */ struct drm_i915_gem_object *batch_obj; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c26d36c..2ebe914 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2758,7 +2758,6 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) while (!list_empty(&ring->request_list)) { struct drm_i915_gem_request *request; - struct intel_ringbuffer *ringbuf; request = list_first_entry(&ring->request_list, struct drm_i915_gem_request, @@ -2769,23 +2768,12 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) trace_i915_gem_request_retire(request); - /* This is one of the few common intersection points - * between legacy ringbuffer submission and execlists: - * we need to tell them apart in order to find the correct - * ringbuffer to which the request belongs to. - */ - if (i915.enable_execlists) { - struct intel_context *ctx = request->ctx; - ringbuf = ctx->engine[ring->id].ringbuf; - } else - ringbuf = ring->buffer; - /* We know the GPU must have read the request to have * sent us the seqno + interrupt, so use the position * of tail of the request to update the last known position * of the GPU head. */ - ringbuf->last_retired_head = request->postfix; + request->ringbuf->last_retired_head = request->postfix; i915_gem_free_request(request); } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 73c1861..762136b 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -878,12 +878,14 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring, return ret; } - /* Hold a reference to the context this request belongs to + /* + * Hold a reference to the context this request belongs to * (we will need it when the time comes to emit/retire the - * request). + * request). Likewise, the ringbuff is useful to keep track of. */ request->ctx = ctx; i915_gem_context_reference(request->ctx); + request->ringbuf = ctx->engine[ring->id].ringbuf; ring->outstanding_lazy_request = request; return 0; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index d611608..ca9e7e6 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2100,6 +2100,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) kref_init(&request->ref); request->ring = ring; + request->ringbuf = ring->buffer; request->uniq = dev_private->request_uniq++; ret = i915_gem_get_seqno(ring->dev, &request->seqno);