Quoting Lionel Landwerlin (2018-05-03 11:13:05) > On 03/05/18 07:36, Chris Wilson wrote: > > Limit the arbitration (where preemption may occur) to inside the batch, > > and prevent it from happening on the pipecontrols/flushes we use to > > write the breadcrumb seqno. Once the user batch is complete, we have > > nothing left to do but serialise and emit the breadcrumb; switching > > contexts at this point is futile so don't. > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx> > > Cc: Michel Thierry <michel.thierry@xxxxxxxxx> > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_lrc.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > index e04798e98db2..70b722c36e65 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -1934,7 +1934,7 @@ static int gen8_emit_bb_start(struct i915_request *rq, > > rq->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(rq->engine); > > } > > > > - cs = intel_ring_begin(rq, 4); > > + cs = intel_ring_begin(rq, 6); > > if (IS_ERR(cs)) > > return PTR_ERR(cs); > > > > @@ -1963,6 +1963,9 @@ static int gen8_emit_bb_start(struct i915_request *rq, > > (flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0); > > *cs++ = lower_32_bits(offset); > > *cs++ = upper_32_bits(offset); > > + > > + *cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE; > > + *cs++ = MI_NOOP; > > intel_ring_advance(rq, cs); > > > > return 0; > > @@ -2105,7 +2108,7 @@ static void gen8_emit_breadcrumb(struct i915_request *request, u32 *cs) > > cs = gen8_emit_ggtt_write(cs, request->global_seqno, > > intel_hws_seqno_address(request->engine)); > > *cs++ = MI_USER_INTERRUPT; > > - *cs++ = MI_NOOP; > > + *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE; > > request->tail = intel_ring_offset(request, cs); > > assert_ring_tail_valid(request->ring, request->tail); > > > > @@ -2121,7 +2124,7 @@ static void gen8_emit_breadcrumb_rcs(struct i915_request *request, u32 *cs) > > cs = gen8_emit_ggtt_write_rcs(cs, request->global_seqno, > > intel_hws_seqno_address(request->engine)); > > *cs++ = MI_USER_INTERRUPT; > > - *cs++ = MI_NOOP; > > + *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE; > > request->tail = intel_ring_offset(request, cs); > > assert_ring_tail_valid(request->ring, request->tail); > > > > Looks good to me. Just one question, you're adding a NOOP in one place, > dropping it in the other 2. What the rational? Writes into the ring have to be in multiples of 2 dwords. It doesn't strictly, as only the RING_TAIL has to be qword aligned and we could fix up the odd dword at the end, but for now the code insists on every packet being an even number of dwords, hence padding with NOPs. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
