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

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

On 11/20/2024 22:39, 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 cbafd42f7f2f..9d569fea8fda 100644
> --- a/drivers/gpu/drm/xe/xe_engine_activity.c
> +++ b/drivers/gpu/drm/xe/xe_engine_activity.c
> @@ -245,6 +245,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;
Doesn't this need to be checking the API version rather than the file 
version? As in "GUC_SUBMIT_VER() >= MAKE_GUC_VER(1, 14, 1)" instead of 
"GUC_FIRMWARE_VER() >= 70.32.0"? One of the major reasons for the v3 
redesign was to support SRIOV VFs. And VFs do not get to see the file 
version, only the API version.

John.

> +
> +	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
> @@ -255,6 +265,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);
>   }
>   
> @@ -268,6 +281,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))
> +		return 0;
> +
>   	return get_engine_total_ticks(guc, hwe);
>   }
>   
> @@ -281,6 +297,9 @@ void xe_engine_activity_enable_stats(struct xe_guc *guc)
>   {
>   	int ret;
>   
> +	if (!guc_engine_activity_supported(guc))
> +		return;
> +
>   	ret = enable_engine_activity_stats(guc);
>   	if (ret)
>   		xe_gt_err(guc_to_gt(guc), "failed to enable activity stats%d\n", ret);
> @@ -307,6 +326,9 @@ int xe_engine_activity_init(struct xe_guc *guc)
>   	struct xe_gt *gt = guc_to_gt(guc);
>   	int ret;
>   
> +	if (!guc_engine_activity_supported(guc))
> +		return 0;
> +
>   	ret = allocate_activity_group(engine_busy);
>   	if (ret) {
>   		xe_gt_err(gt, "failed to allocate activity group %d\n", ret);