[Intel-gfx] [RFC v3 11/11] drm/i915: Gate engine stats collection with a static key

[Tvrtko Ursulin]

On 14/09/2017 21:22, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-09-13 13:18:19)
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> This reduces the cost of the software engine busyness tracking
>> to a single no-op instruction when there are no listeners.
>>
>> v2: Rebase and some comments.
>> v3: Rebase.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_pmu.c         | 54 +++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/intel_engine_cs.c  | 17 ++++++++
>>   drivers/gpu/drm/i915/intel_ringbuffer.h | 70 ++++++++++++++++++++++-----------
>>   3 files changed, 113 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>> index 3c0c5d0549b3..d734879e67ee 100644
>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -460,11 +460,17 @@ static void i915_pmu_enable(struct perf_event *event)
>>                  GEM_BUG_ON(sample >= I915_PMU_SAMPLE_BITS);
>>                  GEM_BUG_ON(engine->pmu.enable_count[sample] == ~0);
>>                  if (engine->pmu.enable_count[sample]++ == 0) {
>> +                       /*
>> +                        * Enable engine busy stats tracking if needed or
>> +                        * alternatively cancel the scheduled disabling of the
>> +                        * same.
>> +                        */
>>                          if (engine_needs_busy_stats(engine) &&
>>                              !engine->pmu.busy_stats) {
>> -                               engine->pmu.busy_stats =
>> -                                       intel_enable_engine_stats(engine) == 0;
>> -                               WARN_ON_ONCE(!engine->pmu.busy_stats);
>> +                               engine->pmu.busy_stats = true;
>> +                               if (!cancel_delayed_work(&engine->pmu.disable_busy_stats))
>> +                                       queue_work(i915->wq,
>> +                                               &engine->pmu.enable_busy_stats);
> 
> The only users of i915->wq are supposed to be dependent on struct_mutex,
> it is limited to a single job under the presumption that each worker
> would be serialised by that mutex.

Good point, so I need a dedicated wq. Might still want to make it 
ordered since they themselves will be serialised by the global static 
key mutex.

Regards,

Tvrtko