Quoting Chris Wilson (2019-10-14 13:34:35)
> Quoting Tvrtko Ursulin (2019-10-14 13:25:58)
> >
> > On 14/10/2019 13:06, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2019-10-14 13:00:01)
> > >>
> > >> On 14/10/2019 10:07, Chris Wilson wrote:
> > >>> +static void cancel_active(struct i915_request *rq,
> > >>> + struct intel_engine_cs *engine)
> > >>> +{
> > >>> + struct intel_context * const ce = rq->hw_context;
> > >>> +
> > >>> + /*
> > >>> + * The executing context has been cancelled. Fixup the context so that
> > >>> + * it will be marked as incomplete [-EIO] upon resubmission and not
> >
> > (read below first)
> >
> > ... and not misleadingly say "Fixup the context so that it will be
> > marked as incomplete" because there is nothing in this function which
> > does that. It mostly happens by the virtual of context already being
> > marked as banned somewhere else. This comment should just explain the
> > decision to rewind the ring->head for more determinism. It can still
> > mention canceling of user payload and -EIO. Just needs to be clear of
> > the single extra thing achieved here by the head rewind and context edit.
>
> I thought I was clear: "upon resubmission". So use a more active voice to
> reduce ambiguity, gotcha.
/*
* The executing context has been cancelled. We want to prevent
* further execution along this context and propagate the error on
* to anything depending on its results.
*
* In __i915_request_submit(), we apply the -EIO and remove the
* requests payload for any banned requests. But first, we must
* rewind the context back to the start of the incomplete request so
* that we don't jump back into the middle of the batch.
*
* We preserve the breadcrumbs and semaphores of the incomplete
* requests so that inter-timeline dependencies (i.e other timelines)
* remain correctly ordered.
*/
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
