drm/i915: Do not end i915 batch buffers prematurely
diff mbox series

Message ID 20191017193719.137439-1-stuart.summers@intel.com
State New
Headers show
Series
  • drm/i915: Do not end i915 batch buffers prematurely
Related show

Commit Message

Stuart Summers Oct. 17, 2019, 7:37 p.m. UTC
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(-)

Comments

Daniele Ceraolo Spurio Oct. 17, 2019, 9:42 p.m. UTC | #1
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);
>
Stuart Summers Oct. 18, 2019, 2:35 p.m. UTC | #2
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);
> >

Patch
diff mbox series

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);