| Message ID | 1426768264-16996-26-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/03/2015 12:30, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > In execlist mode, context initialisation is deferred until first use of the > given context. This is because execlist mode has many more contexts than legacy > mode and many are never actually used. Previously, the initialisation commands Chris Wilson has previously remarked that execlist mode has in fact _no_ more contexts than legacy mode. It's just that we have conflicting definition of terms here. Could you reword this to say "execlist mode has many more context hardware states" or "execlist mode has many more subcontexts" or something? We need to work out how we should refer to these things in a consistent way. > were written to the ring and tagged with some random request structure via the > OLR. This seemed to be causing a null pointer deference bug under certain > circumstances (BZ:88865). > > This patch adds explicit request creation and submission to the deferred > initialisation code path. Thus removing any reliance on or randomness caused by > the OLR. > > Note that it should be possible to move the deferred context creation until even > later - when the context is actually switched to rather than when it is merely > validated. This would allow the initialisation to be done within the request of > the work that is wanting to use the context. Hence, the extra request that is > created, used and retired just for the context init could be removed completely. > However, this is left for a follow up patch. > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index b430e51..c77a74b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1949,13 +1949,22 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > lrc_setup_hardware_status_page(ring, ctx_obj); > else if (ring->id == RCS && !ctx->rcs_initialized) { > if (ring->init_context) { > - ret = ring->init_context(ring, ctx); > + struct drm_i915_gem_request *req; > + > + ret = i915_gem_request_alloc(ring, ctx, &req); > + if (ret) > + return ret; > + > + ret = ring->init_context(req->ring, ctx); > if (ret) { > DRM_ERROR("ring init context: %d\n", ret); > + i915_gem_request_cancel(req); > ctx->engine[ring->id].ringbuf = NULL; > ctx->engine[ring->id].state = NULL; > goto error; > } > + > + i915_add_request_no_flush(req->ring); > } > > ctx->rcs_initialized = true; >
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index b430e51..c77a74b 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1949,13 +1949,22 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, lrc_setup_hardware_status_page(ring, ctx_obj); else if (ring->id == RCS && !ctx->rcs_initialized) { if (ring->init_context) { - ret = ring->init_context(ring, ctx); + struct drm_i915_gem_request *req; + + ret = i915_gem_request_alloc(ring, ctx, &req); + if (ret) + return ret; + + ret = ring->init_context(req->ring, ctx); if (ret) { DRM_ERROR("ring init context: %d\n", ret); + i915_gem_request_cancel(req); ctx->engine[ring->id].ringbuf = NULL; ctx->engine[ring->id].state = NULL; goto error; } + + i915_add_request_no_flush(req->ring); } ctx->rcs_initialized = true;