| Message ID | 20191017193719.137439-1-stuart.summers@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Do not end i915 batch buffers prematurely | expand |
On 10/17/19 12:37 PM, Stuart Summers wrote: > During engine initialization in i915 load, the batch buffers > being used to set up the initial context are being prematurely > ended. In most scenarios, this does not cause a problem, but That's not a batch that we add the BBEND to, that's the context itself. > in the rare event the engine expects the context to be added > without an explicit MI_BATCH_BUFFER_END instruction, do not > insert this instruction prematurely. > We only add the BBEND when there is no state to restore, so why would the engine expect to execute a bunch of no-ops? Daniele > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index e9fe9f79cedd..ec067c29ac65 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -3805,7 +3805,7 @@ populate_lr_context(struct intel_context *ce, > /* The second page of the context object contains some fields which must > * be set up prior to the first execution. */ > regs = vaddr + LRC_STATE_PN * PAGE_SIZE; > - execlists_init_reg_state(regs, ce, engine, ring, inhibit); > + execlists_init_reg_state(regs, ce, engine, ring, false); > if (inhibit) > regs[CTX_CONTEXT_CONTROL] |= > _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT); >
On Thu, 2019-10-17 at 14:42 -0700, Daniele Ceraolo Spurio wrote: > > On 10/17/19 12:37 PM, Stuart Summers wrote: > > During engine initialization in i915 load, the batch buffers > > being used to set up the initial context are being prematurely > > ended. In most scenarios, this does not cause a problem, but > > That's not a batch that we add the BBEND to, that's the context > itself. True. > > > in the rare event the engine expects the context to be added > > without an explicit MI_BATCH_BUFFER_END instruction, do not > > insert this instruction prematurely. > > > > We only add the BBEND when there is no state to restore, so why > would > the engine expect to execute a bunch of no-ops? That is also true. Seems like an unlikely situation. Let me dig a bit more before coming back here. Thanks, Stuart > > Daniele > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c > > b/drivers/gpu/drm/i915/gt/intel_lrc.c > > index e9fe9f79cedd..ec067c29ac65 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > > @@ -3805,7 +3805,7 @@ populate_lr_context(struct intel_context *ce, > > /* The second page of the context object contains some fields > > which must > > * be set up prior to the first execution. */ > > regs = vaddr + LRC_STATE_PN * PAGE_SIZE; > > - execlists_init_reg_state(regs, ce, engine, ring, inhibit); > > + execlists_init_reg_state(regs, ce, engine, ring, false); > > if (inhibit) > > regs[CTX_CONTEXT_CONTROL] |= > > _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_ > > INHIBIT); > >
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index e9fe9f79cedd..ec067c29ac65 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -3805,7 +3805,7 @@ populate_lr_context(struct intel_context *ce, /* The second page of the context object contains some fields which must * be set up prior to the first execution. */ regs = vaddr + LRC_STATE_PN * PAGE_SIZE; - execlists_init_reg_state(regs, ce, engine, ring, inhibit); + execlists_init_reg_state(regs, ce, engine, ring, false); if (inhibit) regs[CTX_CONTEXT_CONTROL] |= _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
During engine initialization in i915 load, the batch buffers being used to set up the initial context are being prematurely ended. In most scenarios, this does not cause a problem, but in the rare event the engine expects the context to be added without an explicit MI_BATCH_BUFFER_END instruction, do not insert this instruction prematurely. Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)