Re: [PATCH v3 08/10] drm/xe/guc: Expose engine activity only for supported GuC version

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

On Mon, Jan 06, 2025 at 01:25:57PM +0530, Riana Tauro wrote:
>Engine activity is supported only on GuC submission version >= 1.14.1
>Allow enabling/reading engine activity only on supported
>GuC versions. Warn once if not supported.
>
>v2: use guc submission version (John)
>
>Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>---
> drivers/gpu/drm/xe/xe_guc_engine_activity.c | 31 +++++++++++++++++++++
> drivers/gpu/drm/xe/xe_guc_engine_activity.h |  1 +
> 2 files changed, 32 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_guc_engine_activity.c b/drivers/gpu/drm/xe/xe_guc_engine_activity.c
>index 14950b42d1b1..5aea42f0cded 100644
>--- a/drivers/gpu/drm/xe/xe_guc_engine_activity.c
>+++ b/drivers/gpu/drm/xe/xe_guc_engine_activity.c
>@@ -255,6 +255,9 @@ u64 xe_guc_engine_activity_active_ticks(struct xe_hw_engine *hwe)
> {
> 	struct xe_guc *guc =  &hwe->gt->uc.guc;
>
>+	if (!xe_guc_engine_activity_supported(guc))
>+		return 0;
>+
> 	return get_engine_active_ticks(guc, hwe);
> }
>
>@@ -268,9 +271,31 @@ u64 xe_guc_engine_activity_total_ticks(struct xe_hw_engine *hwe)
> {
> 	struct xe_guc *guc =  &hwe->gt->uc.guc;
>
>+	if (!xe_guc_engine_activity_supported(guc))
>+		return 0;
>+
> 	return get_engine_total_ticks(guc, hwe);
> }
>
>+/**
>+ * xe_guc_engine_activity_enable_stats - Enable engine activity stats
>+ * @guc: The GuC object
>+ *
>+ * Engine activity stats is supported from GuC submission version
>+ * (1.14.1)
>+ *
>+ * Return: true if engine activity stats supported, false otherwise
>+ */
>+bool xe_guc_engine_activity_supported(struct xe_guc *guc)
>+{
>+	if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 14, 1))
>+		return true;
>+
>+	drm_WARN_ON_ONCE(&guc_to_xe(guc)->drm,
>+			 "Engine busyness not supported in this GuC version\n");

A warn ON may be heavy. Can we live with just a debug print here?

>+	return false;
>+}
>+
> /**
>  * xe_guc_engine_activity_enable_stats - Enable engine activity stats
>  * @guc: The GuC object
>@@ -281,6 +306,9 @@ void xe_guc_engine_activity_enable_stats(struct xe_guc *guc)
> {
> 	int ret;
>
>+	if (!xe_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 +335,9 @@ int xe_guc_engine_activity_init(struct xe_guc *guc)
> 	struct xe_gt *gt = guc_to_gt(guc);
> 	int ret;
>
>+	if (!xe_guc_engine_activity_supported(guc))
>+		return 0;
>+
> 	ret = allocate_engine_activity_group(guc);
> 	if (ret) {
> 		xe_gt_err(gt, "failed to allocate activity group %d\n", ret);
>diff --git a/drivers/gpu/drm/xe/xe_guc_engine_activity.h b/drivers/gpu/drm/xe/xe_guc_engine_activity.h
>index 6cd435b81bbe..e2c2f1bb2b59 100644
>--- a/drivers/gpu/drm/xe/xe_guc_engine_activity.h
>+++ b/drivers/gpu/drm/xe/xe_guc_engine_activity.h
>@@ -12,6 +12,7 @@ struct xe_hw_engine;
> struct xe_guc;
>
> int xe_guc_engine_activity_init(struct xe_guc *guc);
>+bool xe_guc_engine_activity_supported(struct xe_guc *guc);
> void xe_guc_engine_activity_enable_stats(struct xe_guc *guc);
> u64 xe_guc_engine_activity_active_ticks(struct xe_hw_engine *hwe);
> u64 xe_guc_engine_activity_total_ticks(struct xe_hw_engine *hwe);
>-- 
>2.47.1
>