Quoting Tvrtko Ursulin (2018-06-11 11:23:53)
>
> On 11/06/2018 11:04, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-06-11 11:00:31)
> >>
> >> On 10/06/2018 20:43, Chris Wilson wrote:
> >>> @@ -1585,11 +1607,14 @@ static int switch_context(struct i915_request *rq)
> >>>
> >>> to_mm->pd_dirty_rings &= ~intel_engine_flag(engine);
> >>> engine->legacy_active_ppgtt = to_mm;
> >>> - hw_flags = MI_FORCE_RESTORE;
> >>> +
> >>> + if (to_ctx == from_ctx) {
> >>
> >> Contexts can be the same here , when the parent condition is "if (to_mm
> >> != from_mm || to_mm ...)" ?
> >>
> >>> + hw_flags = MI_FORCE_RESTORE;
> >>> + from_ctx = NULL;
> >>
> >> Now on the error path we can end up with engine->legacy_active_context
> >> == NULL, but commands to switch have been put to the ring. Is that OK?
> >
> > On the error path, we unwind the commands in the ring and will overwrite
> > them with the next request (see i915_request_alloc() err_unwind:)
> >
> > So leaving legacy_active_context = NULL here just ensures we emit a
> > context switch next time, no matter what. That is no bad thing as the HW
> > ignores redundant MI_SET_CONTEXT (and requires the FORCE_RESTORE flag to
> > force a redundant reload).
>
> But we don't set MI_FORCE_RESTORE if the old context is equal to new
> one, since we discarded information about the previous one?
Bah, no I was thinking the opposite that we would just end up with more
FORCE_RESTORE. In the next patch, we kill it. Let's just undo all the
unnecessary changes here (can you tell I was avoiding work?)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
