[Intel-gfx] [PATCH] i915/oa: Simplify updating contexts

[Tvrtko Ursulin]

On 12/09/2018 16:50, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-12 16:29:30)
>>          /*
>>           * The OA register config is setup through the context image. This image
>>           * might be written to by the GPU on context switch (in particular on
>> @@ -1833,7 +1727,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
>>           * the GPU from any submitted work.
>>           */
>>          ret = i915_gem_wait_for_idle(dev_priv,
>> -                                    wait_flags,
>> +                                    I915_WAIT_LOCKED,
>>                                       MAX_SCHEDULE_TIMEOUT);
> 
> Wait until idle includes a wait for the gpu to switch off. At least it
> does for execlists, not so clear for ringbuffer as there is no explicit
> idle-event. However, that shouldn't matter as the kernel context doesn't
> exist for legacy ringbuffer anyway ;) But the reload will be forced on
> the next actual use.

And on top this is only called on Gen8+!

>>          if (ret)
>>                  return ret;
>> @@ -1859,7 +1753,17 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
>>                  i915_gem_object_unpin_map(ce->state->obj);
>>          }
>>   
>> -       return ret;
>> +       /*
>> +        * Apply the configuration by doing one context restore of the edited
>> +        * context image.
>> +        */
>> +       rq = i915_request_alloc(engine, dev_priv->kernel_context);
> 
> By feeding a request, you ensure the reconfig is loaded again. +1 for
> having it turn off when idle (and not instrument the kernel context at
> all)!
> 
> Still I follow your logic that this should leave the oa config in
> exactly the same state as before the patch, so
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Thanks, yeah, I am not sure excluding kernel context is possible. If I 
understand the comments in i915_perf.c, and how much Lionel explained to 
me, when on we want it on all the time so sampling timers are always on 
regardless of context switches.

Regards,

Tvrtko