[Intel-gfx] [PATCH 02/17] drm/i915/ringbuffer: Brute force context restore

[Chris Wilson]

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