[Intel-gfx] [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time

Chris Wilson chris at chris-wilson.co.uk
Mon Feb 26 14:52:46 UTC 2018


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


More information about the Intel-gfx mailing list