Message ID | 1461048560-31983-6-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: > The code to switch_mm() is already handled by i915_switch_context(), the > only difference required to setup the aliasing ppgtt is that we need to > emit te switch_mm() on the first context, i.e. when transitioning from > engine->last_context == NULL. Explanation feels a bit short for the amount of change. > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/i915_gem.c | 28 --------------------- > drivers/gpu/drm/i915/i915_gem_context.c | 43 +++++++++------------------------ > drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ---------- > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 - > 5 files changed, 12 insertions(+), 74 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4c55f480f60b..77becfd5a09d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3125,7 +3125,6 @@ int __must_check i915_gem_context_init(struct drm_device *dev); > void i915_gem_context_fini(struct drm_device *dev); > void i915_gem_context_reset(struct drm_device *dev); > int i915_gem_context_open(struct drm_device *dev, struct drm_file *file); > -int i915_gem_context_enable(struct drm_i915_gem_request *req); > void i915_gem_context_close(struct drm_device *dev, struct drm_file *file); > int i915_switch_context(struct drm_i915_gem_request *req); > struct intel_context * > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index fd9a36badb45..4057c0febccd 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4245,34 +4245,6 @@ i915_gem_init_hw(struct drm_device *dev) > } > } > > - /* Now it is safe to go back round and do everything else: */ > - for_each_engine(engine, dev_priv) { > - struct drm_i915_gem_request *req; > - > - req = i915_gem_request_alloc(engine, NULL); > - if (IS_ERR(req)) { > - ret = PTR_ERR(req); > - break; > - } > - > - ret = i915_ppgtt_init_ring(req); > - if (ret) > - goto err_request; > - > - ret = i915_gem_context_enable(req); > - if (ret) > - goto err_request; > - > -err_request: > - i915_add_request_no_flush(req); > - if (ret) { > - DRM_ERROR("Failed to enable %s, error=%d\n", > - engine->name, ret); > - i915_gem_cleanup_engines(dev); > - break; > - } > - } > - > out: > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > return ret; > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 6870556a180b..cf84138de4ec 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -471,27 +471,6 @@ void i915_gem_context_fini(struct drm_device *dev) > dev_priv->kernel_context = NULL; > } > > -int i915_gem_context_enable(struct drm_i915_gem_request *req) > -{ > - struct intel_engine_cs *engine = req->engine; > - int ret; > - > - if (i915.enable_execlists) { > - if (engine->init_context == NULL) > - return 0; > - > - ret = engine->init_context(req); > - } else > - ret = i915_switch_context(req); > - > - if (ret) { > - DRM_ERROR("ring init context: %d\n", ret); > - return ret; > - } > - > - return 0; > -} > - So default context is not switched to at driver load, or before the non-default context even, any more? And that is fine? > static int context_idr_cleanup(int id, void *p, void *data) > { > struct intel_context *ctx = p; > @@ -661,7 +640,7 @@ static bool > needs_pd_load_pre(struct intel_engine_cs *engine, struct intel_context *to) > { > if (!to->ppgtt) > - return false; > + return engine->last_context == NULL; > > if (engine->last_context == to && > !(intel_engine_flag(engine) & to->ppgtt->pd_dirty_rings)) > @@ -724,6 +703,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) > { > struct intel_context *to = req->ctx; > struct intel_engine_cs *engine = req->engine; > + struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt; > struct intel_context *from; > u32 hw_flags; > int ret, i; > @@ -765,7 +745,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) > * Register Immediate commands in Ring Buffer before submitting > * a context."*/ > trace_switch_mm(engine, to); > - ret = to->ppgtt->switch_mm(to->ppgtt, req); > + ret = ppgtt->switch_mm(ppgtt, req); > if (ret) > goto unpin_out; > } > @@ -776,8 +756,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) > * space. This means we must enforce that a page table load > * occur when this occurs. */ > hw_flags = MI_RESTORE_INHIBIT; > - else if (to->ppgtt && > - intel_engine_flag(engine) & to->ppgtt->pd_dirty_rings) > + else if (ppgtt && intel_engine_flag(engine) & ppgtt->pd_dirty_rings) > hw_flags = MI_FORCE_RESTORE; > else > hw_flags = 0; > @@ -822,7 +801,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) > */ > if (needs_pd_load_post(to, hw_flags)) { > trace_switch_mm(engine, to); > - ret = to->ppgtt->switch_mm(to->ppgtt, req); > + ret = ppgtt->switch_mm(to->ppgtt, req); to->ppgtt should be ppgtt I think. > /* The hardware context switch is emitted, but we haven't > * actually changed the state - so it's probably safe to bail > * here. Still, let the user know something dangerous has > @@ -832,8 +811,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) > return ret; > } > > - if (to->ppgtt) > - to->ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); > + if (ppgtt) > + ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); > > for (i = 0; i < MAX_L3_SLICES; i++) { > if (!(to->remap_slice & (1<<i))) > @@ -887,15 +866,17 @@ int i915_switch_context(struct drm_i915_gem_request *req) > struct intel_context *to = req->ctx; > > if (needs_pd_load_pre(engine, to)) { > + struct i915_hw_ppgtt *ppgtt; > int ret; > > + ppgtt = to->ppgtt ?: to_i915(req)->mm.aliasing_ppgtt; Wrong base again. > + > trace_switch_mm(engine, to); > - ret = to->ppgtt->switch_mm(to->ppgtt, req); > + ret = ppgtt->switch_mm(ppgtt, req); > if (ret) > return ret; > > - /* Doing a PD load always reloads the page dirs */ > - to->ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); > + ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); > } > > if (to != engine->last_context) { > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 780e3ad3ca10..50bdba5cb6d2 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2180,19 +2180,6 @@ int i915_ppgtt_init_hw(struct drm_device *dev) > return 0; > } > > -int i915_ppgtt_init_ring(struct drm_i915_gem_request *req) > -{ > - struct i915_hw_ppgtt *ppgtt = to_i915(req)->mm.aliasing_ppgtt; > - > - if (i915.enable_execlists) > - return 0; > - > - if (!ppgtt) > - return 0; > - > - return ppgtt->switch_mm(ppgtt, req); > -} > - > struct i915_hw_ppgtt * > i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv) > { > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index d7dd3d8a8758..333a2fc62b43 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -519,7 +519,6 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev); > > int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); > int i915_ppgtt_init_hw(struct drm_device *dev); > -int i915_ppgtt_init_ring(struct drm_i915_gem_request *req); > void i915_ppgtt_release(struct kref *kref); > struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev, > struct drm_i915_file_private *fpriv); > Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4c55f480f60b..77becfd5a09d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3125,7 +3125,6 @@ int __must_check i915_gem_context_init(struct drm_device *dev); void i915_gem_context_fini(struct drm_device *dev); void i915_gem_context_reset(struct drm_device *dev); int i915_gem_context_open(struct drm_device *dev, struct drm_file *file); -int i915_gem_context_enable(struct drm_i915_gem_request *req); void i915_gem_context_close(struct drm_device *dev, struct drm_file *file); int i915_switch_context(struct drm_i915_gem_request *req); struct intel_context * diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fd9a36badb45..4057c0febccd 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4245,34 +4245,6 @@ i915_gem_init_hw(struct drm_device *dev) } } - /* Now it is safe to go back round and do everything else: */ - for_each_engine(engine, dev_priv) { - struct drm_i915_gem_request *req; - - req = i915_gem_request_alloc(engine, NULL); - if (IS_ERR(req)) { - ret = PTR_ERR(req); - break; - } - - ret = i915_ppgtt_init_ring(req); - if (ret) - goto err_request; - - ret = i915_gem_context_enable(req); - if (ret) - goto err_request; - -err_request: - i915_add_request_no_flush(req); - if (ret) { - DRM_ERROR("Failed to enable %s, error=%d\n", - engine->name, ret); - i915_gem_cleanup_engines(dev); - break; - } - } - out: intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); return ret; diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 6870556a180b..cf84138de4ec 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -471,27 +471,6 @@ void i915_gem_context_fini(struct drm_device *dev) dev_priv->kernel_context = NULL; } -int i915_gem_context_enable(struct drm_i915_gem_request *req) -{ - struct intel_engine_cs *engine = req->engine; - int ret; - - if (i915.enable_execlists) { - if (engine->init_context == NULL) - return 0; - - ret = engine->init_context(req); - } else - ret = i915_switch_context(req); - - if (ret) { - DRM_ERROR("ring init context: %d\n", ret); - return ret; - } - - return 0; -} - static int context_idr_cleanup(int id, void *p, void *data) { struct intel_context *ctx = p; @@ -661,7 +640,7 @@ static bool needs_pd_load_pre(struct intel_engine_cs *engine, struct intel_context *to) { if (!to->ppgtt) - return false; + return engine->last_context == NULL; if (engine->last_context == to && !(intel_engine_flag(engine) & to->ppgtt->pd_dirty_rings)) @@ -724,6 +703,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) { struct intel_context *to = req->ctx; struct intel_engine_cs *engine = req->engine; + struct i915_hw_ppgtt *ppgtt = to->ppgtt ?: req->i915->mm.aliasing_ppgtt; struct intel_context *from; u32 hw_flags; int ret, i; @@ -765,7 +745,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) * Register Immediate commands in Ring Buffer before submitting * a context."*/ trace_switch_mm(engine, to); - ret = to->ppgtt->switch_mm(to->ppgtt, req); + ret = ppgtt->switch_mm(ppgtt, req); if (ret) goto unpin_out; } @@ -776,8 +756,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) * space. This means we must enforce that a page table load * occur when this occurs. */ hw_flags = MI_RESTORE_INHIBIT; - else if (to->ppgtt && - intel_engine_flag(engine) & to->ppgtt->pd_dirty_rings) + else if (ppgtt && intel_engine_flag(engine) & ppgtt->pd_dirty_rings) hw_flags = MI_FORCE_RESTORE; else hw_flags = 0; @@ -822,7 +801,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) */ if (needs_pd_load_post(to, hw_flags)) { trace_switch_mm(engine, to); - ret = to->ppgtt->switch_mm(to->ppgtt, req); + ret = ppgtt->switch_mm(to->ppgtt, req); /* The hardware context switch is emitted, but we haven't * actually changed the state - so it's probably safe to bail * here. Still, let the user know something dangerous has @@ -832,8 +811,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) return ret; } - if (to->ppgtt) - to->ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); + if (ppgtt) + ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); for (i = 0; i < MAX_L3_SLICES; i++) { if (!(to->remap_slice & (1<<i))) @@ -887,15 +866,17 @@ int i915_switch_context(struct drm_i915_gem_request *req) struct intel_context *to = req->ctx; if (needs_pd_load_pre(engine, to)) { + struct i915_hw_ppgtt *ppgtt; int ret; + ppgtt = to->ppgtt ?: to_i915(req)->mm.aliasing_ppgtt; + trace_switch_mm(engine, to); - ret = to->ppgtt->switch_mm(to->ppgtt, req); + ret = ppgtt->switch_mm(ppgtt, req); if (ret) return ret; - /* Doing a PD load always reloads the page dirs */ - to->ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); + ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine); } if (to != engine->last_context) { diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 780e3ad3ca10..50bdba5cb6d2 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2180,19 +2180,6 @@ int i915_ppgtt_init_hw(struct drm_device *dev) return 0; } -int i915_ppgtt_init_ring(struct drm_i915_gem_request *req) -{ - struct i915_hw_ppgtt *ppgtt = to_i915(req)->mm.aliasing_ppgtt; - - if (i915.enable_execlists) - return 0; - - if (!ppgtt) - return 0; - - return ppgtt->switch_mm(ppgtt, req); -} - struct i915_hw_ppgtt * i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv) { diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index d7dd3d8a8758..333a2fc62b43 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -519,7 +519,6 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev); int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); int i915_ppgtt_init_hw(struct drm_device *dev); -int i915_ppgtt_init_ring(struct drm_i915_gem_request *req); void i915_ppgtt_release(struct kref *kref); struct i915_hw_ppgtt *i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv);
The code to switch_mm() is already handled by i915_switch_context(), the only difference required to setup the aliasing ppgtt is that we need to emit te switch_mm() on the first context, i.e. when transitioning from engine->last_context == NULL. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_gem.c | 28 --------------------- drivers/gpu/drm/i915/i915_gem_context.c | 43 +++++++++------------------------ drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ---------- drivers/gpu/drm/i915/i915_gem_gtt.h | 1 - 5 files changed, 12 insertions(+), 74 deletions(-)