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

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Mon, Nov 18, 2024 at 01:07:32PM +0530, Riana Tauro wrote:
>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

Well, I think we shouldn't have to impose that condition on the UMDs - 
read total ticks only if engine busy ticks is non-zero. At the same time 
they should not use a zero delta in the denominator anyways, so you can 
ignore this for now. We can add it if there's an issue later.

Thanks,
Umesh

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