Quoting Chris Wilson (2018-02-26 14:28:36) > Quoting Michał Winiarski (2018-02-26 13:59:59) > > + if (id == RCS) { > > + cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED, > > + intel_hws_preempt_done_address(engine)); > > + } else { > > + cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED, > > + intel_hws_preempt_done_address(engine)); > > + *cs++ = MI_NOOP; > > + *cs++ = MI_NOOP; > > + } > > + *cs++ = MI_USER_INTERRUPT; > > + *cs++ = MI_NOOP; > > + > > + GEM_BUG_ON(!IS_ALIGNED(ring->size, > > + GUC_PREEMPT_BREADCRUMB_BYTES)); > > + GEM_BUG_ON((void *)cs - ring->vaddr != > > + GUC_PREEMPT_BREADCRUMB_BYTES); > > We don't care about these anymore as we don't have to worry about > instructions wrapping. Similarly, we don't need the NOOP padding after > MI_FLUSH_DW. > > Keep setting ring->tail here so we can avoid the hardcoding inside the > submission. That will allow us to adapt this with ease. Quick digression later, intel_lr_context_resume() breaks this idea to keep ring->tail set. So that means we need to keep the fixed size command packet and the hardcoded length. (But we can still remove the asserts as we do not require magic alignments to avoid wraparound issues anymore.) But we do want a comment here for something like GEM_BUG_ON(cs != GUC_PREEMPT_BREADCRUMB_BYTES); to tie the magic constant here to the wq submission. And maybe a comment in inject_preempt_context() to explain that we we submit a command packet that writes GUC_PREEMPT_FINISHED. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx