Re: [PATCH 6/7] drm/xe/guc: Expose engine busyness only for supported GuC version

[Riana Tauro <riana.tauro@intel.com>]

Hi Umesh

On 11/15/2024 5:42 AM, Umesh Nerlige Ramappa wrote:
> On Wed, Nov 13, 2024 at 10:25:48AM +0530, Riana Tauro wrote:
>> Engine busyness is supported only on GuC versions >= 70.32.0
>> Allow enabling/reading engine busyness only on supported
>> GuC versions. Warn once if not supported.
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_engine_activity.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_engine_activity.c b/drivers/gpu/ 
>> drm/xe/xe_engine_activity.c
>> index 7a2ae09d23de..fc34dcf3057a 100644
>> --- a/drivers/gpu/drm/xe/xe_engine_activity.c
>> +++ b/drivers/gpu/drm/xe/xe_engine_activity.c
>> @@ -236,6 +236,16 @@ static u32 gpm_timestamp_shift(struct xe_gt *gt)
>>     return 3 - REG_FIELD_GET(RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK, reg);
>> }
>>
>> +static bool guc_engine_activity_supported(struct xe_guc *guc)
>> +{
>> +    if (GUC_FIRMWARE_VER(guc) >= MAKE_GUC_VER(70, 32, 0))
>> +        return true;
>> +
>> +    drm_WARN_ON_ONCE(&guc_to_xe(guc)->drm,
>> +             "Engine busyness not supported in this GuC version\n");
>> +    return false;
>> +}
>> +
>> /**
>>  * xe_engine_activity_get_active_ticks - Get engine active ticks
>>  * @hwe: The hw_engine object
>> @@ -246,6 +256,9 @@ u64 xe_engine_activity_get_active_ticks(struct 
>> xe_hw_engine *hwe)
>> {
>>     struct xe_guc *guc =  &hwe->gt->uc.guc;
>>
>> +    if (!guc_engine_activity_supported(guc))
>> +        return 0;
>> +
>>     return get_engine_active_ticks(guc, hwe);
>> }
>>
>> @@ -259,6 +272,9 @@ u64 xe_engine_activity_get_total_ticks(struct 
>> xe_hw_engine *hwe)
>> {
>>     struct xe_guc *guc =  &hwe->gt->uc.guc;
>>
>> +    if (!guc_engine_activity_supported(guc))
> 
> Hmm, not sure my response to this landed on the ML. Sending again..
> 
> If GuC is an older version and UMDs have implemented the new interface, 
> we could return CPU time in ticks here so that UMDs will still be able 
> to use something in the denominator instead of a divide by zero. Busy 
> value will still be 0. Thoughts?
We could do this. But shouldn't UMD only read total ticks if they see an 
actual busyness? Busyness % will anyway be 0 even if we return the cpu 
time in ticks

Thanks
Riana Tauro
> 
> Umesh
> 
>> +        return 0;
>> +
>>     return get_engine_total_ticks(guc, hwe);
>> }