| Message ID | 1461048560-31983-8-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/04/16 07:49, Chris Wilson wrote: > We can hide more details of execlists from higher level code by removing > the explicit call to create an execlist context into its first use. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 -------- > drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++++++------- > drivers/gpu/drm/i915/intel_lrc.h | 2 -- > 3 files changed, 14 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 0d210c383487..3da5978ac0f7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1080,14 +1080,6 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, > return ERR_PTR(-EIO); > } > > - if (i915.enable_execlists && !ctx->engine[engine->id].state) { > - int ret = intel_lr_context_deferred_alloc(ctx, engine); > - if (ret) { > - DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret); > - return ERR_PTR(ret); > - } > - } > - > return ctx; > } > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index c104015813d3..b0d20af38574 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -227,6 +227,8 @@ enum { > #define GEN8_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17 > #define GEN9_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x26 > > +static int execlists_context_deferred_alloc(struct intel_context *ctx, > + struct intel_engine_cs *engine); > static int intel_lr_context_pin(struct intel_context *ctx, > struct intel_engine_cs *engine); > > @@ -672,8 +674,6 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request > struct intel_engine_cs *engine = request->engine; > int ret; > > - request->ringbuf = request->ctx->engine[engine->id].ringbuf; > - > if (i915.enable_guc_submission) { > /* > * Check that the GuC has space for the request before > @@ -687,6 +687,14 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request > return ret; > } > > + if (request->ctx->engine[engine->id].state == NULL) { > + ret = execlists_context_deferred_alloc(request->ctx, engine); > + if (ret) > + return ret; > + } > + > + request->ringbuf = request->ctx->engine[engine->id].ringbuf; > + > ret = intel_lr_context_pin(request->ctx, engine); > if (ret) > return ret; > @@ -2091,7 +2099,7 @@ logical_ring_init(struct intel_engine_cs *engine) > if (ret) > goto error; > > - ret = intel_lr_context_deferred_alloc(dctx, engine); > + ret = execlists_context_deferred_alloc(dctx, engine); > if (ret) > goto error; > > @@ -2541,7 +2549,7 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine) > } > > /** > - * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context > + * execlists_context_deferred_alloc() - create the LRC specific bits of a context > * @ctx: LR context to create. > * @ring: engine to be used with the context. > * > @@ -2553,9 +2561,8 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine) > * > * Return: non-zero on error. > */ > - > -int intel_lr_context_deferred_alloc(struct intel_context *ctx, > - struct intel_engine_cs *engine) > +static int execlists_context_deferred_alloc(struct intel_context *ctx, > + struct intel_engine_cs *engine) > { > struct drm_i915_gem_object *ctx_obj; > uint32_t context_size; > diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h > index b17ab79333aa..8bea937973f6 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.h > +++ b/drivers/gpu/drm/i915/intel_lrc.h > @@ -102,8 +102,6 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf, > > void intel_lr_context_free(struct intel_context *ctx); > uint32_t intel_lr_context_size(struct intel_engine_cs *engine); > -int intel_lr_context_deferred_alloc(struct intel_context *ctx, > - struct intel_engine_cs *engine); > void intel_lr_context_unpin(struct intel_context *ctx, > struct intel_engine_cs *engine); > > This one looks fine to me. (And very desirable to improve the layering like this.) Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0d210c383487..3da5978ac0f7 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1080,14 +1080,6 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, return ERR_PTR(-EIO); } - if (i915.enable_execlists && !ctx->engine[engine->id].state) { - int ret = intel_lr_context_deferred_alloc(ctx, engine); - if (ret) { - DRM_DEBUG("Could not create LRC %u: %d\n", ctx_id, ret); - return ERR_PTR(ret); - } - } - return ctx; } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index c104015813d3..b0d20af38574 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -227,6 +227,8 @@ enum { #define GEN8_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17 #define GEN9_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x26 +static int execlists_context_deferred_alloc(struct intel_context *ctx, + struct intel_engine_cs *engine); static int intel_lr_context_pin(struct intel_context *ctx, struct intel_engine_cs *engine); @@ -672,8 +674,6 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request struct intel_engine_cs *engine = request->engine; int ret; - request->ringbuf = request->ctx->engine[engine->id].ringbuf; - if (i915.enable_guc_submission) { /* * Check that the GuC has space for the request before @@ -687,6 +687,14 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request return ret; } + if (request->ctx->engine[engine->id].state == NULL) { + ret = execlists_context_deferred_alloc(request->ctx, engine); + if (ret) + return ret; + } + + request->ringbuf = request->ctx->engine[engine->id].ringbuf; + ret = intel_lr_context_pin(request->ctx, engine); if (ret) return ret; @@ -2091,7 +2099,7 @@ logical_ring_init(struct intel_engine_cs *engine) if (ret) goto error; - ret = intel_lr_context_deferred_alloc(dctx, engine); + ret = execlists_context_deferred_alloc(dctx, engine); if (ret) goto error; @@ -2541,7 +2549,7 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine) } /** - * intel_lr_context_deferred_alloc() - create the LRC specific bits of a context + * execlists_context_deferred_alloc() - create the LRC specific bits of a context * @ctx: LR context to create. * @ring: engine to be used with the context. * @@ -2553,9 +2561,8 @@ uint32_t intel_lr_context_size(struct intel_engine_cs *engine) * * Return: non-zero on error. */ - -int intel_lr_context_deferred_alloc(struct intel_context *ctx, - struct intel_engine_cs *engine) +static int execlists_context_deferred_alloc(struct intel_context *ctx, + struct intel_engine_cs *engine) { struct drm_i915_gem_object *ctx_obj; uint32_t context_size; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index b17ab79333aa..8bea937973f6 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -102,8 +102,6 @@ static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf, void intel_lr_context_free(struct intel_context *ctx); uint32_t intel_lr_context_size(struct intel_engine_cs *engine); -int intel_lr_context_deferred_alloc(struct intel_context *ctx, - struct intel_engine_cs *engine); void intel_lr_context_unpin(struct intel_context *ctx, struct intel_engine_cs *engine);
We can hide more details of execlists from higher level code by removing the explicit call to create an execlist context into its first use. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 -------- drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++++++------- drivers/gpu/drm/i915/intel_lrc.h | 2 -- 3 files changed, 14 insertions(+), 17 deletions(-)