Re: [PATCH 2/3] i915/guc: Ensure busyness counter increases monotonically

[John Harrison <john.c.harrison@intel.com>]

On 11/21/2024 16:31, John Harrison wrote:
> On 11/18/2024 15:22, Umesh Nerlige Ramappa wrote:
>> Active busyness of an engine is calculated using gt timestamp and the
>> context switch in time. While capturing the gt timestamp, it's possible
>> that the context switches out. This race could result in an active
>> busyness value that is greater than the actual context runtime value 
>> by a
>> small amount. This leads to a negative delta and throws off busyness
>> calculations for the user.
>>
>> If a subsequent count is smaller than the previous one, just return the
>> previous one, since we expect the busyness to catch up.
>>
>> Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats 
>> from GuC to pmu")
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
Actually, does this need to be cleared in __reset_guc_busyness_stats()? 
If there is a reset and the live value is returned to zero then you 
won't get any more updates until it gets back to where it used to be.

John.

>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine_types.h      | 5 +++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 ++++-
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> index ba55c059063d..fe1f85e5dda3 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> @@ -343,6 +343,11 @@ struct intel_engine_guc_stats {
>>        * @start_gt_clk: GT clock time of last idle to active transition.
>>        */
>>       u64 start_gt_clk;
>> +
>> +    /**
>> +     * @total: The last value of total returned
>> +     */
>> +    u64 total;
>>   };
>>     union intel_engine_tlb_inv_reg {
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> index c71aedcbce43..56be9f385270 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> @@ -1378,9 +1378,12 @@ static ktime_t guc_engine_busyness(struct 
>> intel_engine_cs *engine, ktime_t *now)
>>           total += intel_gt_clock_interval_to_ns(gt, clk);
>>       }
>>   +    if (total > stats->total)
>> +        stats->total = total;
>> +
>>       spin_unlock_irqrestore(&guc->timestamp.lock, flags);
>>   -    return ns_to_ktime(total);
>> +    return ns_to_ktime(stats->total);
>>   }
>>     static void guc_enable_busyness_worker(struct intel_guc *guc)
>