[Intel-xe] [RFC PATCH 0/3] Add Render C state properties under gpu_idle

[Intel-xe] [RFC PATCH 0/3] Add Render C state properties under gpu_idle

[Riana Tauro]

Add rc_status and rc6_residency_ms under device/gt#/gpu_idle.
Handle counter wrap for the rc6_residency counter and remove existing
files under device/gt#/

Riana Tauro (3):
  drm/xe: add a new sysfs directory for gpu idle properties
  drm/xe : add rc6_residency in ms
  drm/xe/guc_pc : Remove render c state files

 drivers/gpu/drm/xe/Makefile        |   1 +
 drivers/gpu/drm/xe/xe_gt.c         |   5 +
 drivers/gpu/drm/xe/xe_gt_types.h   |   4 +
 drivers/gpu/drm/xe/xe_guc_pc.c     |  58 ----------
 drivers/gpu/drm/xe/xe_idle.c       | 179 +++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_idle.h       |  13 +++
 drivers/gpu/drm/xe/xe_idle_types.h |  32 ++++++
 7 files changed, 234 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_idle.c
 create mode 100644 drivers/gpu/drm/xe/xe_idle.h
 create mode 100644 drivers/gpu/drm/xe/xe_idle_types.h

-- 
2.40.0

[Intel-xe] [RFC PATCH 1/3] drm/xe: add a new sysfs directory for gpu idle properties

[Riana Tauro]

Add a new sysfs directory under devices/gt#/ called gpu_idle
to contain idle properties of GT such as rc_status, rc6_residency.

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
 drivers/gpu/drm/xe/Makefile        |   1 +
 drivers/gpu/drm/xe/xe_gt.c         |   5 +
 drivers/gpu/drm/xe/xe_gt_types.h   |   4 +
 drivers/gpu/drm/xe/xe_idle.c       | 151 +++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_idle.h       |  13 +++
 drivers/gpu/drm/xe/xe_idle_types.h |  32 ++++++
 6 files changed, 206 insertions(+)
 create mode 100644 drivers/gpu/drm/xe/xe_idle.c
 create mode 100644 drivers/gpu/drm/xe/xe_idle.h
 create mode 100644 drivers/gpu/drm/xe/xe_idle_types.h

diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index ee4a95beec20..5d1b837ad001 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -61,6 +61,7 @@ xe-y += xe_bb.o \
 	xe_hw_fence.o \
 	xe_huc.o \
 	xe_huc_debugfs.o \
+	xe_idle.o \
 	xe_irq.o \
 	xe_lrc.o \
 	xe_migrate.o \
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 4186f7f0d42f..be802e6a0054 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -25,6 +25,7 @@
 #include "xe_gt_topology.h"
 #include "xe_guc_engine_types.h"
 #include "xe_hw_fence.h"
+#include "xe_idle.h"
 #include "xe_irq.h"
 #include "xe_lrc.h"
 #include "xe_map.h"
@@ -381,6 +382,10 @@ static int gt_fw_domain_init(struct xe_gt *gt)
 	if (err)
 		goto err_force_wake;
 
+	err = xe_idle_init(&gt->idle);
+	if (err)
+		goto err_force_wake;
+
 	/* XXX: Fake that we pull the engine mask from hwconfig blob */
 	gt->info.engine_mask = gt->info.__engine_mask;
 
diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
index 7c47d67aa8be..1f2a55026c0c 100644
--- a/drivers/gpu/drm/xe/xe_gt_types.h
+++ b/drivers/gpu/drm/xe/xe_gt_types.h
@@ -9,6 +9,7 @@
 #include "xe_force_wake_types.h"
 #include "xe_hw_engine_types.h"
 #include "xe_hw_fence_types.h"
+#include "xe_idle_types.h"
 #include "xe_reg_sr_types.h"
 #include "xe_sa_types.h"
 #include "xe_uc_types.h"
@@ -287,6 +288,9 @@ struct xe_gt {
 	/** @uc: micro controllers on the GT */
 	struct xe_uc uc;
 
+	/** @idle: render c state properties of GT */
+	struct xe_idle idle;
+
 	/** @engine_ops: submission backend engine operations */
 	const struct xe_engine_ops *engine_ops;
 
diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
new file mode 100644
index 000000000000..231bdb45a6b7
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_idle.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include <drm/drm_managed.h>
+#include "regs/xe_gt_regs.h"
+
+#include "xe_device.h"
+#include "xe_gt.h"
+#include "xe_idle.h"
+#include "xe_mmio.h"
+
+/*
+ * Render-C States:
+ * ================
+ *
+ * Render-C states is also a GuC PC feature that is now enabled in Xe for
+ * all platforms.
+ *
+ * RC6 : RC6 is a special power stage which allows the GPU to enter a very
+ * low-voltage mode when idle. This stage is entered automatically when the GPU is idle
+ * when RC6 support is enabled, and as soon as new workload arises
+ * GPU wakes up automatically as well.
+ *
+ * Xe provides a sysfs API for Render-C States:
+ *
+ * device/gt#/gpu_idle/rc* *read-only* files:
+ * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
+ * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
+ *		    Prone to overflows.
+ */
+
+static struct xe_gt *idle_to_gt(struct xe_idle *idle)
+{
+	return container_of(idle, struct xe_gt, idle);
+}
+
+static struct xe_idle *kobj_to_idle(struct kobject *kobj)
+{
+	return container_of(kobj, struct kobj_idle, base)->idle;
+}
+
+static void xe_idle_kobj_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
+static struct kobj_type xe_idle_kobj_type = {
+	.release = xe_idle_kobj_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+};
+
+static ssize_t
+rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	struct xe_idle *idle = kobj_to_idle(kobj);
+	struct xe_gt *gt = idle_to_gt(idle);
+	u32 reg;
+
+	xe_device_mem_access_get(gt_to_xe(gt));
+	reg = xe_mmio_read32(gt, GEN6_GT_CORE_STATUS.reg);
+	xe_device_mem_access_put(gt_to_xe(gt));
+
+	switch (REG_FIELD_GET(RCN_MASK, reg)) {
+	case GT_RC6:
+		return sysfs_emit(buf, "rc6\n");
+	case GT_RC0:
+		return sysfs_emit(buf, "rc0\n");
+	default:
+		return -ENOENT;
+	}
+}
+
+static const struct kobj_attribute rc_status =
+__ATTR(rc_status, 0444, rc_status_show, NULL);
+
+static ssize_t
+rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	struct xe_idle *idle = kobj_to_idle(kobj);
+	struct xe_gt *gt = idle_to_gt(idle);
+	u32 reg;
+	ssize_t ret;
+
+	xe_device_mem_access_get(gt_to_xe(gt));
+	ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+	if (ret)
+		goto out;
+
+	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
+	ret = sysfs_emit(buf, "%u\n", reg);
+
+	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
+out:
+	xe_device_mem_access_put(gt_to_xe(gt));
+	return ret;
+}
+
+static const struct kobj_attribute rc6_residency =
+__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
+
+static const struct attribute *idle_attrs[] = {
+	&rc_status.attr,
+	&rc6_residency.attr,
+	NULL,
+};
+
+static void idle_fini(struct drm_device *drm, void *arg)
+{
+	struct kobj_idle *kobj_idle = arg;
+
+	sysfs_remove_files(&kobj_idle->base, idle_attrs);
+	kobject_put(&kobj_idle->base);
+}
+
+int xe_idle_init(struct xe_idle *idle)
+{
+	struct xe_gt *gt = idle_to_gt(idle);
+	struct xe_device *xe = gt_to_xe(gt);
+	struct kobj_idle *kobj_idle;
+	int err;
+
+	kobj_idle = kzalloc(sizeof(*kobj_idle), GFP_KERNEL);
+
+	if (!kobj_idle)
+		return -ENOMEM;
+
+	err = kobject_init_and_add(&kobj_idle->base, &xe_idle_kobj_type, gt->sysfs, "gpu_idle");
+	if (err)
+		goto exit_kobj;
+
+	kobj_idle->idle = idle;
+
+	err = sysfs_create_files(&kobj_idle->base, idle_attrs);
+	if (err)
+		goto exit_kobj;
+
+	err = drmm_add_action_or_reset(&xe->drm, idle_fini, kobj_idle);
+	if (err)
+		goto exit;
+
+	return 0;
+
+exit:
+	sysfs_remove_files(&kobj_idle->base, idle_attrs);
+
+exit_kobj:
+	kobject_put(&kobj_idle->base);
+	return err;
+}
diff --git a/drivers/gpu/drm/xe/xe_idle.h b/drivers/gpu/drm/xe/xe_idle.h
new file mode 100644
index 000000000000..77928a31cecd
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_idle.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef _XE_IDLE_H_
+#define _XE_IDLE_H_
+
+#include "xe_idle_types.h"
+
+int xe_idle_init(struct xe_idle *idle);
+
+#endif /* _XE_IDLE_H_ */
diff --git a/drivers/gpu/drm/xe/xe_idle_types.h b/drivers/gpu/drm/xe/xe_idle_types.h
new file mode 100644
index 000000000000..94377610c56c
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_idle_types.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef _XE_IDLE_TYPES_H_
+#define _XE_IDLE_TYPES_H_
+
+#include <linux/kobject.h>
+#include <linux/types.h>
+
+/**
+ * struct xe_idle - A struct that contains gpu idle properties
+ */
+struct xe_idle {
+	/** @prev_rc6_residency: previous rc6 residency counter */
+	u64 prev_rc6_residency;
+	/** @cur_rc6_residency: raw driver copy of rc6 residency */
+	u64 cur_rc6_residency;
+};
+
+/**
+ * struct kobj_idle - A kobject struct that connects the kobject and xe_idle
+ */
+struct kobj_idle {
+	/** @base: The actual kobject */
+	struct kobject base;
+	/** @idle: A pointer to the struct xe_idle itself */
+	struct xe_idle *idle;
+};
+#endif /* _XE_IDLE_TYPES_H_ */
+
-- 
2.40.0

[Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Riana Tauro]

add rc6_residency in ms instead of rc6 residency counter.
Handle wrap around for the counter
The counter can still wrap as it relies on the frequency of
counter being read

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
 drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
index 231bdb45a6b7..a7e97ddd7253 100644
--- a/drivers/gpu/drm/xe/xe_idle.c
+++ b/drivers/gpu/drm/xe/xe_idle.c
@@ -11,6 +11,8 @@
 #include "xe_idle.h"
 #include "xe_mmio.h"
 
+#define XE_RC6_MULTIPLIER      1280
+
 /*
  * Render-C States:
  * ================
@@ -27,8 +29,7 @@
  *
  * device/gt#/gpu_idle/rc* *read-only* files:
  * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
- * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
- *		    Prone to overflows.
+ * - rc6_residency_ms: Provide the rc6_residency in ms
  */
 
 static struct xe_gt *idle_to_gt(struct xe_idle *idle)
@@ -51,6 +52,35 @@ static struct kobj_type xe_idle_kobj_type = {
 	.sysfs_ops = &kobj_sysfs_ops,
 };
 
+static u64 rc6_residency_us(struct xe_idle *idle)
+{
+	struct xe_gt *gt = idle_to_gt(idle);
+	u64 cur_residency, delta, overflow_residency, prev_residency;
+
+	overflow_residency = BIT_ULL(32);
+	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
+
+	/*
+	 * Counter wrap handling
+	 * Store previous hw counter values for counter wrap-around handling
+	 * Relying on sufficient frequency of queries otherwise counters can still wrap.
+	 */
+	prev_residency = idle->prev_rc6_residency;
+	idle->prev_rc6_residency = cur_residency;
+
+	/* RC6 delta */
+	if (cur_residency >= prev_residency)
+		delta = cur_residency - prev_residency;
+	else
+		delta = cur_residency + (overflow_residency - prev_residency);
+
+	/* Add delta to RC6 extended raw driver copy. */
+	cur_residency = idle->cur_rc6_residency + delta;
+	idle->cur_rc6_residency = cur_residency;
+
+	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
+}
+
 static ssize_t
 rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
 {
@@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
 __ATTR(rc_status, 0444, rc_status_show, NULL);
 
 static ssize_t
-rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
 {
 	struct xe_idle *idle = kobj_to_idle(kobj);
 	struct xe_gt *gt = idle_to_gt(idle);
-	u32 reg;
 	ssize_t ret;
 
 	xe_device_mem_access_get(gt_to_xe(gt));
@@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
 	if (ret)
 		goto out;
 
-	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
-	ret = sysfs_emit(buf, "%u\n", reg);
+	ret = sysfs_emit(buf, "%llu\n", DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
 
 	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
 out:
@@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
 	return ret;
 }
 
-static const struct kobj_attribute rc6_residency =
-__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
+static const struct kobj_attribute rc6_residency_ms =
+__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
 
 static const struct attribute *idle_attrs[] = {
 	&rc_status.attr,
-	&rc6_residency.attr,
+	&rc6_residency_ms.attr,
 	NULL,
 };
 
-- 
2.40.0

[Intel-xe] [RFC PATCH 3/3] drm/xe/guc_pc : Remove render c state files

[Riana Tauro]

Render c state sysfs entries are moved under device/gt#/gpu_idle.
Remove the files under device/gt#/

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
 drivers/gpu/drm/xe/xe_guc_pc.c | 58 ----------------------------------
 1 file changed, 58 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
index 0b6d0577a8a7..0e1e196174ad 100644
--- a/drivers/gpu/drm/xe/xe_guc_pc.c
+++ b/drivers/gpu/drm/xe/xe_guc_pc.c
@@ -71,17 +71,6 @@
  * - freq_max: GuC PC max request.
  *             If max <= min, then freq_min becomes a fixed frequency request.
  *
- * Render-C States:
- * ================
- *
- * Render-C states is also a GuC PC feature that is now enabled in Xe for
- * all platforms.
- * Xe's GuC PC provides a sysfs API for Render-C States:
- *
- * device/gt#/rc* *read-only* files:
- * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
- * - rc6_residency: Provide the rc6_residency counter in units of 1.28 uSec.
- *                  Prone to overflows.
  */
 
 static struct xe_guc *
@@ -580,51 +569,6 @@ static ssize_t freq_max_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(freq_max);
 
-static ssize_t rc_status_show(struct device *dev,
-			      struct device_attribute *attr, char *buff)
-{
-	struct xe_guc_pc *pc = dev_to_pc(dev);
-	struct xe_gt *gt = pc_to_gt(pc);
-	u32 reg;
-
-	xe_device_mem_access_get(gt_to_xe(gt));
-	reg = xe_mmio_read32(gt, GEN6_GT_CORE_STATUS.reg);
-	xe_device_mem_access_put(gt_to_xe(gt));
-
-	switch (REG_FIELD_GET(RCN_MASK, reg)) {
-	case GT_RC6:
-		return sysfs_emit(buff, "rc6\n");
-	case GT_RC0:
-		return sysfs_emit(buff, "rc0\n");
-	default:
-		return -ENOENT;
-	}
-}
-static DEVICE_ATTR_RO(rc_status);
-
-static ssize_t rc6_residency_show(struct device *dev,
-				  struct device_attribute *attr, char *buff)
-{
-	struct xe_guc_pc *pc = dev_to_pc(dev);
-	struct xe_gt *gt = pc_to_gt(pc);
-	u32 reg;
-	ssize_t ret;
-
-	xe_device_mem_access_get(pc_to_xe(pc));
-	ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
-	if (ret)
-		goto out;
-
-	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
-	ret = sysfs_emit(buff, "%u\n", reg);
-
-	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
-out:
-	xe_device_mem_access_put(pc_to_xe(pc));
-	return ret;
-}
-static DEVICE_ATTR_RO(rc6_residency);
-
 static const struct attribute *pc_attrs[] = {
 	&dev_attr_freq_act.attr,
 	&dev_attr_freq_cur.attr,
@@ -633,8 +577,6 @@ static const struct attribute *pc_attrs[] = {
 	&dev_attr_freq_rpn.attr,
 	&dev_attr_freq_min.attr,
 	&dev_attr_freq_max.attr,
-	&dev_attr_rc_status.attr,
-	&dev_attr_rc6_residency.attr,
 	NULL
 };
 
-- 
2.40.0

[Intel-xe] ✗ CI.Patch_applied: failure for Add Render C state properties under gpu_idle

[Patchwork]

== Series Details ==

Series: Add Render C state properties under gpu_idle
URL   : https://patchwork.freedesktop.org/series/117222/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-xe-next' with base: ===
Base commit: 9f096ce76 drm/xe: Enable Raptorlake-P
=== git am output follows ===
.git/rebase-apply/patch:283: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
error: patch failed: drivers/gpu/drm/xe/xe_guc_pc.c:580
error: drivers/gpu/drm/xe/xe_guc_pc.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch
Applying: drm/xe: add a new sysfs directory for gpu idle properties
Applying: drm/xe : add rc6_residency in ms
Applying: drm/xe/guc_pc : Remove render c state files
Patch failed at 0003 drm/xe/guc_pc : Remove render c state files
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [Intel-xe] [RFC PATCH 1/3] drm/xe: add a new sysfs directory for gpu idle properties

[Gupta, Anshuman]

> -----Original Message-----
> From: Tauro, Riana <riana.tauro@intel.com>
> Sent: Wednesday, May 3, 2023 12:13 PM
> To: intel-xe@lists.freedesktop.org
> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
> Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>
> Subject: [RFC PATCH 1/3] drm/xe: add a new sysfs directory for gpu idle
> properties
> 
> Add a new sysfs directory under devices/gt#/ called gpu_idle to contain idle
> properties of GT such as rc_status, rc6_residency.
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/Makefile        |   1 +
>  drivers/gpu/drm/xe/xe_gt.c         |   5 +
>  drivers/gpu/drm/xe/xe_gt_types.h   |   4 +
>  drivers/gpu/drm/xe/xe_idle.c       | 151
> +++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_idle.h       |  13 +++
>  drivers/gpu/drm/xe/xe_idle_types.h |  32 ++++++
>  6 files changed, 206 insertions(+)
>  create mode 100644 drivers/gpu/drm/xe/xe_idle.c  create mode 100644
> drivers/gpu/drm/xe/xe_idle.h  create mode 100644
> drivers/gpu/drm/xe/xe_idle_types.h
> 
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index ee4a95beec20..5d1b837ad001 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -61,6 +61,7 @@ xe-y += xe_bb.o \
>  	xe_hw_fence.o \
>  	xe_huc.o \
>  	xe_huc_debugfs.o \
> +	xe_idle.o \
>  	xe_irq.o \
>  	xe_lrc.o \
>  	xe_migrate.o \
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index
> 4186f7f0d42f..be802e6a0054 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -25,6 +25,7 @@
>  #include "xe_gt_topology.h"
>  #include "xe_guc_engine_types.h"
>  #include "xe_hw_fence.h"
> +#include "xe_idle.h"
>  #include "xe_irq.h"
>  #include "xe_lrc.h"
>  #include "xe_map.h"
> @@ -381,6 +382,10 @@ static int gt_fw_domain_init(struct xe_gt *gt)
>  	if (err)
>  		goto err_force_wake;
> 
> +	err = xe_idle_init(&gt->idle);
> +	if (err)
> +		goto err_force_wake;
> +
>  	/* XXX: Fake that we pull the engine mask from hwconfig blob */
>  	gt->info.engine_mask = gt->info.__engine_mask;
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h
> b/drivers/gpu/drm/xe/xe_gt_types.h
> index 7c47d67aa8be..1f2a55026c0c 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -9,6 +9,7 @@
>  #include "xe_force_wake_types.h"
>  #include "xe_hw_engine_types.h"
>  #include "xe_hw_fence_types.h"
> +#include "xe_idle_types.h"
>  #include "xe_reg_sr_types.h"
>  #include "xe_sa_types.h"
>  #include "xe_uc_types.h"
> @@ -287,6 +288,9 @@ struct xe_gt {
>  	/** @uc: micro controllers on the GT */
>  	struct xe_uc uc;
> 
> +	/** @idle: render c state properties of GT */
> +	struct xe_idle idle;
> +
>  	/** @engine_ops: submission backend engine operations */
>  	const struct xe_engine_ops *engine_ops;
> 
> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> new file mode 100644 index 000000000000..231bdb45a6b7
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_idle.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <drm/drm_managed.h>
> +#include "regs/xe_gt_regs.h"
> +
> +#include "xe_device.h"
> +#include "xe_gt.h"
> +#include "xe_idle.h"
> +#include "xe_mmio.h"
> +
> +/*
> + * Render-C States:
> + * ================
> + *
> + * Render-C states is also a GuC PC feature that is now enabled in Xe
> +for
> + * all platforms.
> + *
> + * RC6 : RC6 is a special power stage which allows the GPU to enter a
> +very
> + * low-voltage mode when idle. This stage is entered automatically when
> +the GPU is idle
> + * when RC6 support is enabled, and as soon as new workload arises
> + * GPU wakes up automatically as well.
> + *
> + * Xe provides a sysfs API for Render-C States:
> + *
> + * device/gt#/gpu_idle/rc* *read-only* files:
> + * - rc_status: Provide the actual immediate status of Render-C: (rc0
> +or rc6)
> + * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> + *		    Prone to overflows.
> + */
> +
> +static struct xe_gt *idle_to_gt(struct xe_idle *idle) {
> +	return container_of(idle, struct xe_gt, idle); }
> +
> +static struct xe_idle *kobj_to_idle(struct kobject *kobj) {
> +	return container_of(kobj, struct kobj_idle, base)->idle; }
> +
> +static void xe_idle_kobj_release(struct kobject *kobj) {
> +	kfree(kobj);
> +}
> +
> +static struct kobj_type xe_idle_kobj_type = {
> +	.release = xe_idle_kobj_release,
> +	.sysfs_ops = &kobj_sysfs_ops,
> +};
> +
> +static ssize_t
> +rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char
> +*buf) {
> +	struct xe_idle *idle = kobj_to_idle(kobj);
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u32 reg;
> +
> +	xe_device_mem_access_get(gt_to_xe(gt));
> +	reg = xe_mmio_read32(gt, GEN6_GT_CORE_STATUS.reg);
> +	xe_device_mem_access_put(gt_to_xe(gt));
> +
> +	switch (REG_FIELD_GET(RCN_MASK, reg)) {
> +	case GT_RC6:
> +		return sysfs_emit(buf, "rc6\n");
> +	case GT_RC0:
> +		return sysfs_emit(buf, "rc0\n");
> +	default:
> +		return -ENOENT;
> +	}
> +}
> +
> +static const struct kobj_attribute rc_status = __ATTR(rc_status, 0444,
> +rc_status_show, NULL);
> +
> +static ssize_t
> +rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr,
> +char *buf) {
> +	struct xe_idle *idle = kobj_to_idle(kobj);
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u32 reg;
> +	ssize_t ret;
> +
> +	xe_device_mem_access_get(gt_to_xe(gt));
> +	ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	if (ret)
> +		goto out;
> +
> +	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> +	ret = sysfs_emit(buf, "%u\n", reg);
> +
> +	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
> XE_FORCEWAKE_ALL));
> +out:
> +	xe_device_mem_access_put(gt_to_xe(gt));
> +	return ret;
> +}
> +
> +static const struct kobj_attribute rc6_residency =
> +__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
How about calling it idle_residency_ms to make it agnostic for media c6 residency as well ?
@Rodrigo any input here ?
Br,
Anshuman Gupta.
> +
> +static const struct attribute *idle_attrs[] = {
> +	&rc_status.attr,
> +	&rc6_residency.attr,
> +	NULL,
> +};
> +
> +static void idle_fini(struct drm_device *drm, void *arg) {
> +	struct kobj_idle *kobj_idle = arg;
> +
> +	sysfs_remove_files(&kobj_idle->base, idle_attrs);
> +	kobject_put(&kobj_idle->base);
> +}
> +
> +int xe_idle_init(struct xe_idle *idle)
> +{
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	struct xe_device *xe = gt_to_xe(gt);
> +	struct kobj_idle *kobj_idle;
> +	int err;
> +
> +	kobj_idle = kzalloc(sizeof(*kobj_idle), GFP_KERNEL);
> +
> +	if (!kobj_idle)
> +		return -ENOMEM;
> +
> +	err = kobject_init_and_add(&kobj_idle->base, &xe_idle_kobj_type,
> gt->sysfs, "gpu_idle");
> +	if (err)
> +		goto exit_kobj;
> +
> +	kobj_idle->idle = idle;
> +
> +	err = sysfs_create_files(&kobj_idle->base, idle_attrs);
> +	if (err)
> +		goto exit_kobj;
> +
> +	err = drmm_add_action_or_reset(&xe->drm, idle_fini, kobj_idle);
> +	if (err)
> +		goto exit;
> +
> +	return 0;
> +
> +exit:
> +	sysfs_remove_files(&kobj_idle->base, idle_attrs);
> +
> +exit_kobj:
> +	kobject_put(&kobj_idle->base);
> +	return err;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_idle.h b/drivers/gpu/drm/xe/xe_idle.h
> new file mode 100644 index 000000000000..77928a31cecd
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_idle.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_IDLE_H_
> +#define _XE_IDLE_H_
> +
> +#include "xe_idle_types.h"
> +
> +int xe_idle_init(struct xe_idle *idle);
> +
> +#endif /* _XE_IDLE_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_idle_types.h
> b/drivers/gpu/drm/xe/xe_idle_types.h
> new file mode 100644
> index 000000000000..94377610c56c
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_idle_types.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_IDLE_TYPES_H_
> +#define _XE_IDLE_TYPES_H_
> +
> +#include <linux/kobject.h>
> +#include <linux/types.h>
> +
> +/**
> + * struct xe_idle - A struct that contains gpu idle properties  */
> +struct xe_idle {
> +	/** @prev_rc6_residency: previous rc6 residency counter */
> +	u64 prev_rc6_residency;
> +	/** @cur_rc6_residency: raw driver copy of rc6 residency */
> +	u64 cur_rc6_residency;
> +};
> +
> +/**
> + * struct kobj_idle - A kobject struct that connects the kobject and
> +xe_idle  */ struct kobj_idle {
> +	/** @base: The actual kobject */
> +	struct kobject base;
> +	/** @idle: A pointer to the struct xe_idle itself */
> +	struct xe_idle *idle;
> +};
> +#endif /* _XE_IDLE_TYPES_H_ */
> +
> --
> 2.40.0

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Gupta, Anshuman]

> -----Original Message-----
> From: Tauro, Riana <riana.tauro@intel.com>
> Sent: Wednesday, May 3, 2023 12:13 PM
> To: intel-xe@lists.freedesktop.org
> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
> Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>
> Subject: [RFC PATCH 2/3] drm/xe : add rc6_residency in ms
> 
> add rc6_residency in ms instead of rc6 residency counter.
> Handle wrap around for the counter
> The counter can still wrap as it relies on the frequency of counter being read
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++----
> ---
>  1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> index 231bdb45a6b7..a7e97ddd7253 100644
> --- a/drivers/gpu/drm/xe/xe_idle.c
> +++ b/drivers/gpu/drm/xe/xe_idle.c
> @@ -11,6 +11,8 @@
>  #include "xe_idle.h"
>  #include "xe_mmio.h"
> 
> +#define XE_RC6_MULTIPLIER      1280
> +
>  /*
>   * Render-C States:
>   * ================
> @@ -27,8 +29,7 @@
>   *
>   * device/gt#/gpu_idle/rc* *read-only* files:
>   * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
> - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> - *		    Prone to overflows.
> + * - rc6_residency_ms: Provide the rc6_residency in ms
>   */
> 
>  static struct xe_gt *idle_to_gt(struct xe_idle *idle) @@ -51,6 +52,35 @@
> static struct kobj_type xe_idle_kobj_type = {
>  	.sysfs_ops = &kobj_sysfs_ops,
>  };
> 
> +static u64 rc6_residency_us(struct xe_idle *idle) {
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u64 cur_residency, delta, overflow_residency, prev_residency;
> +
> +	overflow_residency = BIT_ULL(32);
> +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
We shall use function pointer here to get the residency, so underlying function can read from
appropriate counter and  abstract it from sysfs show function, same comment for converting to 
ms as well.
Br,
Anshuman Gupta.
> +
> +	/*
> +	 * Counter wrap handling
> +	 * Store previous hw counter values for counter wrap-around
> handling
> +	 * Relying on sufficient frequency of queries otherwise counters can
> still wrap.
> +	 */
> +	prev_residency = idle->prev_rc6_residency;
> +	idle->prev_rc6_residency = cur_residency;
> +
> +	/* RC6 delta */
> +	if (cur_residency >= prev_residency)
> +		delta = cur_residency - prev_residency;
> +	else
> +		delta = cur_residency + (overflow_residency -
> prev_residency);
> +
> +	/* Add delta to RC6 extended raw driver copy. */
> +	cur_residency = idle->cur_rc6_residency + delta;
> +	idle->cur_rc6_residency = cur_residency;
> +
> +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
> }
> +
>  static ssize_t
>  rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> { @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
> __ATTR(rc_status, 0444, rc_status_show, NULL);
> 
>  static ssize_t
> -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char
> *buf)
> +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute
> +*attr, char *buf)
>  {
>  	struct xe_idle *idle = kobj_to_idle(kobj);
>  	struct xe_gt *gt = idle_to_gt(idle);
> -	u32 reg;
>  	ssize_t ret;
> 
>  	xe_device_mem_access_get(gt_to_xe(gt));
> @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct
> kobj_attribute *attr, char *buf)
>  	if (ret)
>  		goto out;
> 
> -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> -	ret = sysfs_emit(buf, "%u\n", reg);
> +	ret = sysfs_emit(buf, "%llu\n",
> +DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
> 
>  	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
> XE_FORCEWAKE_ALL));
>  out:
> @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct
> kobj_attribute *attr, char *buf)
>  	return ret;
>  }
> 
> -static const struct kobj_attribute rc6_residency = -__ATTR(rc6_residency,
> 0444, rc6_residency_show, NULL);
> +static const struct kobj_attribute rc6_residency_ms =
> +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
> 
>  static const struct attribute *idle_attrs[] = {
>  	&rc_status.attr,
> -	&rc6_residency.attr,
> +	&rc6_residency_ms.attr,
>  	NULL,
>  };
> 
> --
> 2.40.0

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Nilawar, Badal]

Hi Riana,

On 03-05-2023 12:12, Riana Tauro wrote:
> add rc6_residency in ms instead of rc6 residency counter.
> Handle wrap around for the counter
> The counter can still wrap as it relies on the frequency of
> counter being read
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++-------
>   1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> index 231bdb45a6b7..a7e97ddd7253 100644
> --- a/drivers/gpu/drm/xe/xe_idle.c
> +++ b/drivers/gpu/drm/xe/xe_idle.c
> @@ -11,6 +11,8 @@
>   #include "xe_idle.h"
>   #include "xe_mmio.h"
>   
> +#define XE_RC6_MULTIPLIER      1280
> +
>   /*
>    * Render-C States:
>    * ================
> @@ -27,8 +29,7 @@
>    *
>    * device/gt#/gpu_idle/rc* *read-only* files:
>    * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
> - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> - *		    Prone to overflows.
> + * - rc6_residency_ms: Provide the rc6_residency in ms
>    */
>   
>   static struct xe_gt *idle_to_gt(struct xe_idle *idle)
> @@ -51,6 +52,35 @@ static struct kobj_type xe_idle_kobj_type = {
>   	.sysfs_ops = &kobj_sysfs_ops,
>   };
>   
> +static u64 rc6_residency_us(struct xe_idle *idle)
> +{
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u64 cur_residency, delta, overflow_residency, prev_residency;
> +
> +	overflow_residency = BIT_ULL(32);
> +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
GEN6_GT_GFX is punit register so does not fall any of the forcewake 
domain. So the xe_force_wake_get() in the function rc6_residency_ms_show 
can be avoided.
> +
> +	/*
> +	 * Counter wrap handling
> +	 * Store previous hw counter values for counter wrap-around handling
> +	 * Relying on sufficient frequency of queries otherwise counters can still wrap.
> +	 */
> +	prev_residency = idle->prev_rc6_residency;
> +	idle->prev_rc6_residency = cur_residency;
> +
> +	/* RC6 delta */
> +	if (cur_residency >= prev_residency)
> +		delta = cur_residency - prev_residency;
> +	else
> +		delta = cur_residency + (overflow_residency - prev_residency);
> +
> +	/* Add delta to RC6 extended raw driver copy. */
> +	cur_residency = idle->cur_rc6_residency + delta;
> +	idle->cur_rc6_residency = cur_residency;
> +
> +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
> +}
> +
>   static ssize_t
>   rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>   {
> @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
>   __ATTR(rc_status, 0444, rc_status_show, NULL);
>   
>   static ssize_t
> -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>   {
>   	struct xe_idle *idle = kobj_to_idle(kobj);
>   	struct xe_gt *gt = idle_to_gt(idle);
> -	u32 reg;
>   	ssize_t ret;
>   
>   	xe_device_mem_access_get(gt_to_xe(gt));
> @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>   	if (ret)
>   		goto out;
>   
> -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> -	ret = sysfs_emit(buf, "%u\n", reg);
> +	ret = sysfs_emit(buf, "%llu\n", DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
>   
>   	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
>   out:
> @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>   	return ret;
>   }
>   
> -static const struct kobj_attribute rc6_residency =
> -__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
> +static const struct kobj_attribute rc6_residency_ms =
> +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
__ATTR_RO or DEVICE_ATTR_RO would be more appropriate here.

Regards,
Badal
>   
>   static const struct attribute *idle_attrs[] = {
>   	&rc_status.attr,
> -	&rc6_residency.attr,
> +	&rc6_residency_ms.attr,
>   	NULL,
>   };
>   

Re: [Intel-xe] [RFC PATCH 1/3] drm/xe: add a new sysfs directory for gpu idle properties

[Nilawar, Badal]

Hi Riana,

On 03-05-2023 12:12, Riana Tauro wrote:
> Add a new sysfs directory under devices/gt#/ called gpu_idle
> to contain idle properties of GT such as rc_status, rc6_residency.
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>   drivers/gpu/drm/xe/Makefile        |   1 +
>   drivers/gpu/drm/xe/xe_gt.c         |   5 +
>   drivers/gpu/drm/xe/xe_gt_types.h   |   4 +
>   drivers/gpu/drm/xe/xe_idle.c       | 151 +++++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_idle.h       |  13 +++
>   drivers/gpu/drm/xe/xe_idle_types.h |  32 ++++++
>   6 files changed, 206 insertions(+)
>   create mode 100644 drivers/gpu/drm/xe/xe_idle.c
>   create mode 100644 drivers/gpu/drm/xe/xe_idle.h
>   create mode 100644 drivers/gpu/drm/xe/xe_idle_types.h
> 
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index ee4a95beec20..5d1b837ad001 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -61,6 +61,7 @@ xe-y += xe_bb.o \
>   	xe_hw_fence.o \
>   	xe_huc.o \
>   	xe_huc_debugfs.o \
> +	xe_idle.o \
>   	xe_irq.o \
>   	xe_lrc.o \
>   	xe_migrate.o \
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 4186f7f0d42f..be802e6a0054 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -25,6 +25,7 @@
>   #include "xe_gt_topology.h"
>   #include "xe_guc_engine_types.h"
>   #include "xe_hw_fence.h"
> +#include "xe_idle.h"
>   #include "xe_irq.h"
>   #include "xe_lrc.h"
>   #include "xe_map.h"
> @@ -381,6 +382,10 @@ static int gt_fw_domain_init(struct xe_gt *gt)
>   	if (err)
>   		goto err_force_wake;
>   
> +	err = xe_idle_init(&gt->idle);
> +	if (err)
> +		goto err_force_wake;
> +
>   	/* XXX: Fake that we pull the engine mask from hwconfig blob */
>   	gt->info.engine_mask = gt->info.__engine_mask;
>   
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index 7c47d67aa8be..1f2a55026c0c 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -9,6 +9,7 @@
>   #include "xe_force_wake_types.h"
>   #include "xe_hw_engine_types.h"
>   #include "xe_hw_fence_types.h"
> +#include "xe_idle_types.h"
>   #include "xe_reg_sr_types.h"
>   #include "xe_sa_types.h"
>   #include "xe_uc_types.h"
> @@ -287,6 +288,9 @@ struct xe_gt {
>   	/** @uc: micro controllers on the GT */
>   	struct xe_uc uc;
>   
> +	/** @idle: render c state properties of GT */
> +	struct xe_idle idle;
> +
>   	/** @engine_ops: submission backend engine operations */
>   	const struct xe_engine_ops *engine_ops;
>   
> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> new file mode 100644
> index 000000000000..231bdb45a6b7
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_idle.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <drm/drm_managed.h>
> +#include "regs/xe_gt_regs.h"
> +
> +#include "xe_device.h"
> +#include "xe_gt.h"
> +#include "xe_idle.h"
> +#include "xe_mmio.h"
> +
> +/*
> + * Render-C States:
> + * ================
> + *
> + * Render-C states is also a GuC PC feature that is now enabled in Xe for
> + * all platforms.
> + *
> + * RC6 : RC6 is a special power stage which allows the GPU to enter a very
> + * low-voltage mode when idle. This stage is entered automatically when the GPU is idle
> + * when RC6 support is enabled, and as soon as new workload arises
> + * GPU wakes up automatically as well.
> + *
> + * Xe provides a sysfs API for Render-C States:
> + *
> + * device/gt#/gpu_idle/rc* *read-only* files:
> + * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
> + * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> + *		    Prone to overflows.
> + */
> +
> +static struct xe_gt *idle_to_gt(struct xe_idle *idle)
> +{
> +	return container_of(idle, struct xe_gt, idle);
> +}
> +
> +static struct xe_idle *kobj_to_idle(struct kobject *kobj)
> +{
> +	return container_of(kobj, struct kobj_idle, base)->idle;
> +}
> +
> +static void xe_idle_kobj_release(struct kobject *kobj)
> +{
> +	kfree(kobj);
> +}
> +
> +static struct kobj_type xe_idle_kobj_type = {
> +	.release = xe_idle_kobj_release,
> +	.sysfs_ops = &kobj_sysfs_ops,
> +};
> +
> +static ssize_t
> +rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> +	struct xe_idle *idle = kobj_to_idle(kobj);
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u32 reg;
> +
> +	xe_device_mem_access_get(gt_to_xe(gt));
> +	reg = xe_mmio_read32(gt, GEN6_GT_CORE_STATUS.reg);
> +	xe_device_mem_access_put(gt_to_xe(gt));This can be abstracted by adding variable xe_reg to xe_idle structure, 
to hold GT_CORE_STATUS reg, which can be initialized xe_idle_init 
function. > +
> +	switch (REG_FIELD_GET(RCN_MASK, reg)) {
> +	case GT_RC6:
> +		return sysfs_emit(buf, "rc6\n");
> +	case GT_RC0:
> +		return sysfs_emit(buf, "rc0\n");
> +	default:
> +		return -ENOENT;
> +	}
> +}
> +
> +static const struct kobj_attribute rc_status =
> +__ATTR(rc_status, 0444, rc_status_show, NULL);
> +
> +static ssize_t
> +rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> +	struct xe_idle *idle = kobj_to_idle(kobj);
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u32 reg;
> +	ssize_t ret;
> +
> +	xe_device_mem_access_get(gt_to_xe(gt));
> +	ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	if (ret)
> +		goto out;
> +
> +	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
As mentioned in other patch this register does not fall under any of the 
forcewake domain so forcewake can be avoided here.
Abstraction is applicable here has well.
> +	ret = sysfs_emit(buf, "%u\n", reg);
> +
> +	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
> +out:
> +	xe_device_mem_access_put(gt_to_xe(gt));
> +	return ret;
> +}
> +
> +static const struct kobj_attribute rc6_residency =
> +__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
Good to use __ATTR_RO or DEV_ATTR_RO here.

Regards,
Badal
> +
> +static const struct attribute *idle_attrs[] = {
> +	&rc_status.attr,
> +	&rc6_residency.attr,
> +	NULL,
> +};
> +
> +static void idle_fini(struct drm_device *drm, void *arg)
> +{
> +	struct kobj_idle *kobj_idle = arg;
> +
> +	sysfs_remove_files(&kobj_idle->base, idle_attrs);
> +	kobject_put(&kobj_idle->base);
> +}
> +
> +int xe_idle_init(struct xe_idle *idle)
> +{
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	struct xe_device *xe = gt_to_xe(gt);
> +	struct kobj_idle *kobj_idle;
> +	int err;
> +
> +	kobj_idle = kzalloc(sizeof(*kobj_idle), GFP_KERNEL);
> +
> +	if (!kobj_idle)
> +		return -ENOMEM;
> +
> +	err = kobject_init_and_add(&kobj_idle->base, &xe_idle_kobj_type, gt->sysfs, "gpu_idle");
> +	if (err)
> +		goto exit_kobj;
> +
> +	kobj_idle->idle = idle;
> +
> +	err = sysfs_create_files(&kobj_idle->base, idle_attrs);
> +	if (err)
> +		goto exit_kobj;
> +
> +	err = drmm_add_action_or_reset(&xe->drm, idle_fini, kobj_idle);
> +	if (err)
> +		goto exit;
> +
> +	return 0;
> +
> +exit:
> +	sysfs_remove_files(&kobj_idle->base, idle_attrs);
> +
> +exit_kobj:
> +	kobject_put(&kobj_idle->base);
> +	return err;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_idle.h b/drivers/gpu/drm/xe/xe_idle.h
> new file mode 100644
> index 000000000000..77928a31cecd
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_idle.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_IDLE_H_
> +#define _XE_IDLE_H_
> +
> +#include "xe_idle_types.h"
> +
> +int xe_idle_init(struct xe_idle *idle);
> +
> +#endif /* _XE_IDLE_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_idle_types.h b/drivers/gpu/drm/xe/xe_idle_types.h
> new file mode 100644
> index 000000000000..94377610c56c
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_idle_types.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_IDLE_TYPES_H_
> +#define _XE_IDLE_TYPES_H_
> +
> +#include <linux/kobject.h>
> +#include <linux/types.h>
> +
> +/**
> + * struct xe_idle - A struct that contains gpu idle properties
> + */
> +struct xe_idle {
> +	/** @prev_rc6_residency: previous rc6 residency counter */
> +	u64 prev_rc6_residency;
> +	/** @cur_rc6_residency: raw driver copy of rc6 residency */
> +	u64 cur_rc6_residency;
> +};
> +
> +/**
> + * struct kobj_idle - A kobject struct that connects the kobject and xe_idle
> + */
> +struct kobj_idle {
> +	/** @base: The actual kobject */
> +	struct kobject base;
> +	/** @idle: A pointer to the struct xe_idle itself */
> +	struct xe_idle *idle;
> +};
> +#endif /* _XE_IDLE_TYPES_H_ */
> +

Re: [Intel-xe] [RFC PATCH 1/3] drm/xe: add a new sysfs directory for gpu idle properties

[Rodrigo Vivi]

On Thu, May 04, 2023 at 06:51:59AM -0400, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Tauro, Riana <riana.tauro@intel.com>
> > Sent: Wednesday, May 3, 2023 12:13 PM
> > To: intel-xe@lists.freedesktop.org
> > Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> > <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
> > Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> > <badal.nilawar@intel.com>
> > Subject: [RFC PATCH 1/3] drm/xe: add a new sysfs directory for gpu idle
> > properties
> > 
> > Add a new sysfs directory under devices/gt#/ called gpu_idle to contain idle
> > properties of GT such as rc_status, rc6_residency.
> > 
> > Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> > ---
> >  drivers/gpu/drm/xe/Makefile        |   1 +
> >  drivers/gpu/drm/xe/xe_gt.c         |   5 +
> >  drivers/gpu/drm/xe/xe_gt_types.h   |   4 +
> >  drivers/gpu/drm/xe/xe_idle.c       | 151
> > +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_idle.h       |  13 +++
> >  drivers/gpu/drm/xe/xe_idle_types.h |  32 ++++++
> >  6 files changed, 206 insertions(+)
> >  create mode 100644 drivers/gpu/drm/xe/xe_idle.c  create mode 100644
> > drivers/gpu/drm/xe/xe_idle.h  create mode 100644
> > drivers/gpu/drm/xe/xe_idle_types.h
> > 
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index ee4a95beec20..5d1b837ad001 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -61,6 +61,7 @@ xe-y += xe_bb.o \
> >  	xe_hw_fence.o \
> >  	xe_huc.o \
> >  	xe_huc_debugfs.o \
> > +	xe_idle.o \
> >  	xe_irq.o \
> >  	xe_lrc.o \
> >  	xe_migrate.o \
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index
> > 4186f7f0d42f..be802e6a0054 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -25,6 +25,7 @@
> >  #include "xe_gt_topology.h"
> >  #include "xe_guc_engine_types.h"
> >  #include "xe_hw_fence.h"
> > +#include "xe_idle.h"
> >  #include "xe_irq.h"
> >  #include "xe_lrc.h"
> >  #include "xe_map.h"
> > @@ -381,6 +382,10 @@ static int gt_fw_domain_init(struct xe_gt *gt)
> >  	if (err)
> >  		goto err_force_wake;
> > 
> > +	err = xe_idle_init(&gt->idle);
> > +	if (err)
> > +		goto err_force_wake;
> > +
> >  	/* XXX: Fake that we pull the engine mask from hwconfig blob */
> >  	gt->info.engine_mask = gt->info.__engine_mask;
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_types.h
> > b/drivers/gpu/drm/xe/xe_gt_types.h
> > index 7c47d67aa8be..1f2a55026c0c 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_types.h
> > +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> > @@ -9,6 +9,7 @@
> >  #include "xe_force_wake_types.h"
> >  #include "xe_hw_engine_types.h"
> >  #include "xe_hw_fence_types.h"
> > +#include "xe_idle_types.h"
> >  #include "xe_reg_sr_types.h"
> >  #include "xe_sa_types.h"
> >  #include "xe_uc_types.h"
> > @@ -287,6 +288,9 @@ struct xe_gt {
> >  	/** @uc: micro controllers on the GT */
> >  	struct xe_uc uc;
> > 
> > +	/** @idle: render c state properties of GT */
> > +	struct xe_idle idle;
> > +
> >  	/** @engine_ops: submission backend engine operations */
> >  	const struct xe_engine_ops *engine_ops;
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> > new file mode 100644 index 000000000000..231bdb45a6b7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_idle.c
> > @@ -0,0 +1,151 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#include <drm/drm_managed.h>
> > +#include "regs/xe_gt_regs.h"
> > +
> > +#include "xe_device.h"
> > +#include "xe_gt.h"
> > +#include "xe_idle.h"
> > +#include "xe_mmio.h"
> > +
> > +/*
> > + * Render-C States:
> > + * ================
> > + *
> > + * Render-C states is also a GuC PC feature that is now enabled in Xe
> > +for
> > + * all platforms.
> > + *
> > + * RC6 : RC6 is a special power stage which allows the GPU to enter a
> > +very
> > + * low-voltage mode when idle. This stage is entered automatically when
> > +the GPU is idle
> > + * when RC6 support is enabled, and as soon as new workload arises
> > + * GPU wakes up automatically as well.
> > + *
> > + * Xe provides a sysfs API for Render-C States:
> > + *
> > + * device/gt#/gpu_idle/rc* *read-only* files:
> > + * - rc_status: Provide the actual immediate status of Render-C: (rc0
> > +or rc6)
> > + * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> > + *		    Prone to overflows.
> > + */
> > +
> > +static struct xe_gt *idle_to_gt(struct xe_idle *idle) {
> > +	return container_of(idle, struct xe_gt, idle); }
> > +
> > +static struct xe_idle *kobj_to_idle(struct kobject *kobj) {
> > +	return container_of(kobj, struct kobj_idle, base)->idle; }
> > +
> > +static void xe_idle_kobj_release(struct kobject *kobj) {
> > +	kfree(kobj);
> > +}
> > +
> > +static struct kobj_type xe_idle_kobj_type = {
> > +	.release = xe_idle_kobj_release,
> > +	.sysfs_ops = &kobj_sysfs_ops,
> > +};
> > +
> > +static ssize_t
> > +rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char
> > +*buf) {
> > +	struct xe_idle *idle = kobj_to_idle(kobj);
> > +	struct xe_gt *gt = idle_to_gt(idle);
> > +	u32 reg;
> > +
> > +	xe_device_mem_access_get(gt_to_xe(gt));
> > +	reg = xe_mmio_read32(gt, GEN6_GT_CORE_STATUS.reg);
> > +	xe_device_mem_access_put(gt_to_xe(gt));
> > +
> > +	switch (REG_FIELD_GET(RCN_MASK, reg)) {
> > +	case GT_RC6:
> > +		return sysfs_emit(buf, "rc6\n");
> > +	case GT_RC0:
> > +		return sysfs_emit(buf, "rc0\n");
> > +	default:
> > +		return -ENOENT;
> > +	}
> > +}
> > +
> > +static const struct kobj_attribute rc_status = __ATTR(rc_status, 0444,
> > +rc_status_show, NULL);
> > +
> > +static ssize_t
> > +rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr,
> > +char *buf) {
> > +	struct xe_idle *idle = kobj_to_idle(kobj);
> > +	struct xe_gt *gt = idle_to_gt(idle);
> > +	u32 reg;
> > +	ssize_t ret;
> > +
> > +	xe_device_mem_access_get(gt_to_xe(gt));
> > +	ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> > +	if (ret)
> > +		goto out;
> > +
> > +	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> > +	ret = sysfs_emit(buf, "%u\n", reg);
> > +
> > +	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
> > XE_FORCEWAKE_ALL));
> > +out:
> > +	xe_device_mem_access_put(gt_to_xe(gt));
> > +	return ret;
> > +}
> > +
> > +static const struct kobj_attribute rc6_residency =
> > +__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
> How about calling it idle_residency_ms to make it agnostic for media c6 residency as well ?
> @Rodrigo any input here ?

what a tough question!

A few thoughts in random order:

* if we are going with xe_idle then probably idle_residency_ms is probably a good call.
* I kinda like 'idle' because it aligns with cpuidle...
* I kind of dislike 'idle' because we abstract to much the meaning of it...
* I like that 'idle' fixes the confusion with media... Media-RC6 vs MC6 vs MediaC6 vs whatever...
* maybe 'gtidle' (xe_gt_idle?) instead of just idle?! grep for idle in xe code and you see vm_idle,
runtime_idle, execlist_port_idle, engine_idle... what idle are we talking about when
we have simply xe_idle?
* Maybe a more generic 'idle', but probably with 'idle<n>' directories infra where you have
  underneath stuff like:
  + 'name' (gt0-rc6, gt1-rc6, media0, display?, ...)
  + 'residency_us'
  + 'state' (this could even be turned into a knob to enable/disable the feature at runtime)

> Br,
> Anshuman Gupta.
> > +
> > +static const struct attribute *idle_attrs[] = {
> > +	&rc_status.attr,
> > +	&rc6_residency.attr,
> > +	NULL,
> > +};
> > +
> > +static void idle_fini(struct drm_device *drm, void *arg) {
> > +	struct kobj_idle *kobj_idle = arg;
> > +
> > +	sysfs_remove_files(&kobj_idle->base, idle_attrs);
> > +	kobject_put(&kobj_idle->base);
> > +}
> > +
> > +int xe_idle_init(struct xe_idle *idle)
> > +{
> > +	struct xe_gt *gt = idle_to_gt(idle);
> > +	struct xe_device *xe = gt_to_xe(gt);
> > +	struct kobj_idle *kobj_idle;
> > +	int err;
> > +
> > +	kobj_idle = kzalloc(sizeof(*kobj_idle), GFP_KERNEL);
> > +
> > +	if (!kobj_idle)
> > +		return -ENOMEM;
> > +
> > +	err = kobject_init_and_add(&kobj_idle->base, &xe_idle_kobj_type,
> > gt->sysfs, "gpu_idle");
> > +	if (err)
> > +		goto exit_kobj;
> > +
> > +	kobj_idle->idle = idle;
> > +
> > +	err = sysfs_create_files(&kobj_idle->base, idle_attrs);
> > +	if (err)
> > +		goto exit_kobj;
> > +
> > +	err = drmm_add_action_or_reset(&xe->drm, idle_fini, kobj_idle);
> > +	if (err)
> > +		goto exit;
> > +
> > +	return 0;
> > +
> > +exit:
> > +	sysfs_remove_files(&kobj_idle->base, idle_attrs);
> > +
> > +exit_kobj:
> > +	kobject_put(&kobj_idle->base);
> > +	return err;
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_idle.h b/drivers/gpu/drm/xe/xe_idle.h
> > new file mode 100644 index 000000000000..77928a31cecd
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_idle.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_IDLE_H_
> > +#define _XE_IDLE_H_
> > +
> > +#include "xe_idle_types.h"
> > +
> > +int xe_idle_init(struct xe_idle *idle);
> > +
> > +#endif /* _XE_IDLE_H_ */
> > diff --git a/drivers/gpu/drm/xe/xe_idle_types.h
> > b/drivers/gpu/drm/xe/xe_idle_types.h
> > new file mode 100644
> > index 000000000000..94377610c56c
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_idle_types.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_IDLE_TYPES_H_
> > +#define _XE_IDLE_TYPES_H_
> > +
> > +#include <linux/kobject.h>
> > +#include <linux/types.h>
> > +
> > +/**
> > + * struct xe_idle - A struct that contains gpu idle properties  */
> > +struct xe_idle {
> > +	/** @prev_rc6_residency: previous rc6 residency counter */
> > +	u64 prev_rc6_residency;
> > +	/** @cur_rc6_residency: raw driver copy of rc6 residency */
> > +	u64 cur_rc6_residency;
> > +};
> > +
> > +/**
> > + * struct kobj_idle - A kobject struct that connects the kobject and
> > +xe_idle  */ struct kobj_idle {
> > +	/** @base: The actual kobject */
> > +	struct kobject base;
> > +	/** @idle: A pointer to the struct xe_idle itself */
> > +	struct xe_idle *idle;
> > +};
> > +#endif /* _XE_IDLE_TYPES_H_ */
> > +
> > --
> > 2.40.0
> 

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Rodrigo Vivi]

On Thu, May 04, 2023 at 06:55:48AM -0400, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Tauro, Riana <riana.tauro@intel.com>
> > Sent: Wednesday, May 3, 2023 12:13 PM
> > To: intel-xe@lists.freedesktop.org
> > Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> > <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
> > Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> > <badal.nilawar@intel.com>
> > Subject: [RFC PATCH 2/3] drm/xe : add rc6_residency in ms
> > 
> > add rc6_residency in ms instead of rc6 residency counter.
> > Handle wrap around for the counter
> > The counter can still wrap as it relies on the frequency of counter being read
> > 
> > Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++----
> > ---
> >  1 file changed, 37 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> > index 231bdb45a6b7..a7e97ddd7253 100644
> > --- a/drivers/gpu/drm/xe/xe_idle.c
> > +++ b/drivers/gpu/drm/xe/xe_idle.c
> > @@ -11,6 +11,8 @@
> >  #include "xe_idle.h"
> >  #include "xe_mmio.h"
> > 
> > +#define XE_RC6_MULTIPLIER      1280
> > +
> >  /*
> >   * Render-C States:
> >   * ================
> > @@ -27,8 +29,7 @@
> >   *
> >   * device/gt#/gpu_idle/rc* *read-only* files:
> >   * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
> > - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> > - *		    Prone to overflows.
> > + * - rc6_residency_ms: Provide the rc6_residency in ms
> >   */
> > 
> >  static struct xe_gt *idle_to_gt(struct xe_idle *idle) @@ -51,6 +52,35 @@
> > static struct kobj_type xe_idle_kobj_type = {
> >  	.sysfs_ops = &kobj_sysfs_ops,
> >  };
> > 
> > +static u64 rc6_residency_us(struct xe_idle *idle) {
> > +	struct xe_gt *gt = idle_to_gt(idle);
> > +	u64 cur_residency, delta, overflow_residency, prev_residency;
> > +
> > +	overflow_residency = BIT_ULL(32);
> > +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> We shall use function pointer here to get the residency, so underlying function can read from
> appropriate counter and  abstract it from sysfs show function, same comment for converting to 
> ms as well.

Yes, please!

Leave this new component as clean and free from the 'RC' stuff as possible.
Leave all RC stuff inside xe_guc_pc and then this new infra is specific for
the report and control so you can make this as generic as possible and likely
extend to other stuff.
And contain the platform stuff to the lower level.

> Br,
> Anshuman Gupta.
> > +
> > +	/*
> > +	 * Counter wrap handling
> > +	 * Store previous hw counter values for counter wrap-around
> > handling
> > +	 * Relying on sufficient frequency of queries otherwise counters can
> > still wrap.
> > +	 */
> > +	prev_residency = idle->prev_rc6_residency;
> > +	idle->prev_rc6_residency = cur_residency;
> > +
> > +	/* RC6 delta */
> > +	if (cur_residency >= prev_residency)
> > +		delta = cur_residency - prev_residency;
> > +	else
> > +		delta = cur_residency + (overflow_residency -
> > prev_residency);
> > +
> > +	/* Add delta to RC6 extended raw driver copy. */
> > +	cur_residency = idle->cur_rc6_residency + delta;
> > +	idle->cur_rc6_residency = cur_residency;
> > +
> > +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
> > }
> > +
> >  static ssize_t
> >  rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> > { @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
> > __ATTR(rc_status, 0444, rc_status_show, NULL);
> > 
> >  static ssize_t
> > -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char
> > *buf)
> > +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute
> > +*attr, char *buf)
> >  {
> >  	struct xe_idle *idle = kobj_to_idle(kobj);
> >  	struct xe_gt *gt = idle_to_gt(idle);
> > -	u32 reg;
> >  	ssize_t ret;
> > 
> >  	xe_device_mem_access_get(gt_to_xe(gt));
> > @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct
> > kobj_attribute *attr, char *buf)
> >  	if (ret)
> >  		goto out;
> > 
> > -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> > -	ret = sysfs_emit(buf, "%u\n", reg);
> > +	ret = sysfs_emit(buf, "%llu\n",
> > +DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
> > 
> >  	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
> > XE_FORCEWAKE_ALL));
> >  out:
> > @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct
> > kobj_attribute *attr, char *buf)
> >  	return ret;
> >  }
> > 
> > -static const struct kobj_attribute rc6_residency = -__ATTR(rc6_residency,
> > 0444, rc6_residency_show, NULL);
> > +static const struct kobj_attribute rc6_residency_ms =
> > +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
> > 
> >  static const struct attribute *idle_attrs[] = {
> >  	&rc_status.attr,
> > -	&rc6_residency.attr,
> > +	&rc6_residency_ms.attr,
> >  	NULL,
> >  };
> > 
> > --
> > 2.40.0
> 

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Rodrigo Vivi]

On Wed, May 03, 2023 at 12:12:51PM +0530, Riana Tauro wrote:
> add rc6_residency in ms instead of rc6 residency counter.
> Handle wrap around for the counter
> The counter can still wrap as it relies on the frequency of
> counter being read
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> index 231bdb45a6b7..a7e97ddd7253 100644
> --- a/drivers/gpu/drm/xe/xe_idle.c
> +++ b/drivers/gpu/drm/xe/xe_idle.c
> @@ -11,6 +11,8 @@
>  #include "xe_idle.h"
>  #include "xe_mmio.h"
>  
> +#define XE_RC6_MULTIPLIER      1280
> +
>  /*
>   * Render-C States:
>   * ================
> @@ -27,8 +29,7 @@
>   *
>   * device/gt#/gpu_idle/rc* *read-only* files:
>   * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
> - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> - *		    Prone to overflows.
> + * - rc6_residency_ms: Provide the rc6_residency in ms

Ack on the change to _ms, but let's do keep this in xe_guc_pc for now.

>   */
>  
>  static struct xe_gt *idle_to_gt(struct xe_idle *idle)
> @@ -51,6 +52,35 @@ static struct kobj_type xe_idle_kobj_type = {
>  	.sysfs_ops = &kobj_sysfs_ops,
>  };
>  
> +static u64 rc6_residency_us(struct xe_idle *idle)
> +{
> +	struct xe_gt *gt = idle_to_gt(idle);
> +	u64 cur_residency, delta, overflow_residency, prev_residency;
> +
> +	overflow_residency = BIT_ULL(32);
> +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> +
> +	/*
> +	 * Counter wrap handling
> +	 * Store previous hw counter values for counter wrap-around handling
> +	 * Relying on sufficient frequency of queries otherwise counters can still wrap.
> +	 */
> +	prev_residency = idle->prev_rc6_residency;
> +	idle->prev_rc6_residency = cur_residency;
> +
> +	/* RC6 delta */
> +	if (cur_residency >= prev_residency)
> +		delta = cur_residency - prev_residency;
> +	else
> +		delta = cur_residency + (overflow_residency - prev_residency);
> +
> +	/* Add delta to RC6 extended raw driver copy. */
> +	cur_residency = idle->cur_rc6_residency + delta;
> +	idle->cur_rc6_residency = cur_residency;
> +
> +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
> +}
> +
>  static ssize_t
>  rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>  {
> @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
>  __ATTR(rc_status, 0444, rc_status_show, NULL);
>  
>  static ssize_t
> -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>  {
>  	struct xe_idle *idle = kobj_to_idle(kobj);
>  	struct xe_gt *gt = idle_to_gt(idle);
> -	u32 reg;
>  	ssize_t ret;
>  
>  	xe_device_mem_access_get(gt_to_xe(gt));
> @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>  	if (ret)
>  		goto out;
>  
> -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> -	ret = sysfs_emit(buf, "%u\n", reg);
> +	ret = sysfs_emit(buf, "%llu\n", DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
>  
>  	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
>  out:
> @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>  	return ret;
>  }
>  
> -static const struct kobj_attribute rc6_residency =
> -__ATTR(rc6_residency, 0444, rc6_residency_show, NULL);
> +static const struct kobj_attribute rc6_residency_ms =
> +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
>  
>  static const struct attribute *idle_attrs[] = {
>  	&rc_status.attr,
> -	&rc6_residency.attr,
> +	&rc6_residency_ms.attr,
>  	NULL,
>  };
>  
> -- 
> 2.40.0
> 

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Riana Tauro]

On 5/4/2023 9:27 PM, Rodrigo Vivi wrote:
> On Thu, May 04, 2023 at 06:55:48AM -0400, Gupta, Anshuman wrote:
>>
>>
>>> -----Original Message-----
>>> From: Tauro, Riana <riana.tauro@intel.com>
>>> Sent: Wednesday, May 3, 2023 12:13 PM
>>> To: intel-xe@lists.freedesktop.org
>>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
>>> Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
>>> <badal.nilawar@intel.com>
>>> Subject: [RFC PATCH 2/3] drm/xe : add rc6_residency in ms
>>>
>>> add rc6_residency in ms instead of rc6 residency counter.
>>> Handle wrap around for the counter
>>> The counter can still wrap as it relies on the frequency of counter being read
>>>
>>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++----
>>> ---
>>>   1 file changed, 37 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
>>> index 231bdb45a6b7..a7e97ddd7253 100644
>>> --- a/drivers/gpu/drm/xe/xe_idle.c
>>> +++ b/drivers/gpu/drm/xe/xe_idle.c
>>> @@ -11,6 +11,8 @@
>>>   #include "xe_idle.h"
>>>   #include "xe_mmio.h"
>>>
>>> +#define XE_RC6_MULTIPLIER      1280
>>> +
>>>   /*
>>>    * Render-C States:
>>>    * ================
>>> @@ -27,8 +29,7 @@
>>>    *
>>>    * device/gt#/gpu_idle/rc* *read-only* files:
>>>    * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
>>> - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
>>> - *		    Prone to overflows.
>>> + * - rc6_residency_ms: Provide the rc6_residency in ms
>>>    */
>>>
>>>   static struct xe_gt *idle_to_gt(struct xe_idle *idle) @@ -51,6 +52,35 @@
>>> static struct kobj_type xe_idle_kobj_type = {
>>>   	.sysfs_ops = &kobj_sysfs_ops,
>>>   };
>>>
>>> +static u64 rc6_residency_us(struct xe_idle *idle) {
>>> +	struct xe_gt *gt = idle_to_gt(idle);
>>> +	u64 cur_residency, delta, overflow_residency, prev_residency;
>>> +
>>> +	overflow_residency = BIT_ULL(32);
>>> +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
>> We shall use function pointer here to get the residency, so underlying function can read from
>> appropriate counter and  abstract it from sysfs show function, same comment for converting to
>> ms as well.
> 
> Yes, please!
> 
> Leave this new component as clean and free from the 'RC' stuff as possible.
> Leave all RC stuff inside xe_guc_pc and then this new infra is specific for
> the report and control so you can make this as generic as possible and likely
> extend to other stuff.
> And contain the platform stuff to the lower level.
Hi Rodrigo

So do we have two entries for residency? rc6_residency under 
device/gt#/rc and also displayed as idle_residency under 
device/gt#/gpuidle?

> 
>> Br,
>> Anshuman Gupta.
>>> +
>>> +	/*
>>> +	 * Counter wrap handling
>>> +	 * Store previous hw counter values for counter wrap-around
>>> handling
>>> +	 * Relying on sufficient frequency of queries otherwise counters can
>>> still wrap.
>>> +	 */
>>> +	prev_residency = idle->prev_rc6_residency;
>>> +	idle->prev_rc6_residency = cur_residency;
>>> +
>>> +	/* RC6 delta */
>>> +	if (cur_residency >= prev_residency)
>>> +		delta = cur_residency - prev_residency;
>>> +	else
>>> +		delta = cur_residency + (overflow_residency -
>>> prev_residency);
>>> +
>>> +	/* Add delta to RC6 extended raw driver copy. */
>>> +	cur_residency = idle->cur_rc6_residency + delta;
>>> +	idle->cur_rc6_residency = cur_residency;
>>> +
>>> +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
>>> }
>>> +
>>>   static ssize_t
>>>   rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>>> { @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
>>> __ATTR(rc_status, 0444, rc_status_show, NULL);
>>>
>>>   static ssize_t
>>> -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char
>>> *buf)
>>> +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute
>>> +*attr, char *buf)
>>>   {
>>>   	struct xe_idle *idle = kobj_to_idle(kobj);
>>>   	struct xe_gt *gt = idle_to_gt(idle);
>>> -	u32 reg;
>>>   	ssize_t ret;
>>>
>>>   	xe_device_mem_access_get(gt_to_xe(gt));
>>> @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct
>>> kobj_attribute *attr, char *buf)
>>>   	if (ret)
>>>   		goto out;
>>>
>>> -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
>>> -	ret = sysfs_emit(buf, "%u\n", reg);
>>> +	ret = sysfs_emit(buf, "%llu\n",
>>> +DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
>>>
>>>   	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
>>> XE_FORCEWAKE_ALL));
>>>   out:
>>> @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct
>>> kobj_attribute *attr, char *buf)
>>>   	return ret;
>>>   }
>>>
>>> -static const struct kobj_attribute rc6_residency = -__ATTR(rc6_residency,
>>> 0444, rc6_residency_show, NULL);
>>> +static const struct kobj_attribute rc6_residency_ms =
>>> +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
>>>
>>>   static const struct attribute *idle_attrs[] = {
>>>   	&rc_status.attr,
>>> -	&rc6_residency.attr,
>>> +	&rc6_residency_ms.attr,
>>>   	NULL,
>>>   };
>>>
>>> --
>>> 2.40.0
>>

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Rodrigo Vivi]

On Fri, May 05, 2023 at 01:14:29PM +0530, Riana Tauro wrote:
> 
> 
> On 5/4/2023 9:27 PM, Rodrigo Vivi wrote:
> > On Thu, May 04, 2023 at 06:55:48AM -0400, Gupta, Anshuman wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Tauro, Riana <riana.tauro@intel.com>
> > > > Sent: Wednesday, May 3, 2023 12:13 PM
> > > > To: intel-xe@lists.freedesktop.org
> > > > Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
> > > > <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
> > > > Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
> > > > <badal.nilawar@intel.com>
> > > > Subject: [RFC PATCH 2/3] drm/xe : add rc6_residency in ms
> > > > 
> > > > add rc6_residency in ms instead of rc6 residency counter.
> > > > Handle wrap around for the counter
> > > > The counter can still wrap as it relies on the frequency of counter being read
> > > > 
> > > > Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++----
> > > > ---
> > > >   1 file changed, 37 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
> > > > index 231bdb45a6b7..a7e97ddd7253 100644
> > > > --- a/drivers/gpu/drm/xe/xe_idle.c
> > > > +++ b/drivers/gpu/drm/xe/xe_idle.c
> > > > @@ -11,6 +11,8 @@
> > > >   #include "xe_idle.h"
> > > >   #include "xe_mmio.h"
> > > > 
> > > > +#define XE_RC6_MULTIPLIER      1280
> > > > +
> > > >   /*
> > > >    * Render-C States:
> > > >    * ================
> > > > @@ -27,8 +29,7 @@
> > > >    *
> > > >    * device/gt#/gpu_idle/rc* *read-only* files:
> > > >    * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
> > > > - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
> > > > - *		    Prone to overflows.
> > > > + * - rc6_residency_ms: Provide the rc6_residency in ms
> > > >    */
> > > > 
> > > >   static struct xe_gt *idle_to_gt(struct xe_idle *idle) @@ -51,6 +52,35 @@
> > > > static struct kobj_type xe_idle_kobj_type = {
> > > >   	.sysfs_ops = &kobj_sysfs_ops,
> > > >   };
> > > > 
> > > > +static u64 rc6_residency_us(struct xe_idle *idle) {
> > > > +	struct xe_gt *gt = idle_to_gt(idle);
> > > > +	u64 cur_residency, delta, overflow_residency, prev_residency;
> > > > +
> > > > +	overflow_residency = BIT_ULL(32);
> > > > +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> > > We shall use function pointer here to get the residency, so underlying function can read from
> > > appropriate counter and  abstract it from sysfs show function, same comment for converting to
> > > ms as well.
> > 
> > Yes, please!
> > 
> > Leave this new component as clean and free from the 'RC' stuff as possible.
> > Leave all RC stuff inside xe_guc_pc and then this new infra is specific for
> > the report and control so you can make this as generic as possible and likely
> > extend to other stuff.
> > And contain the platform stuff to the lower level.
> Hi Rodrigo
> 
> So do we have two entries for residency? rc6_residency under device/gt#/rc
> and also displayed as idle_residency under device/gt#/gpuidle?

No, that was not what I meant. I'm sorry for not being clear.

We should have only one interface. Let's not duplicate stuff and minimize the interfaces.

What I tried to say was to leave all the RC6 stuff inside xe_guc_pc... All RC6 related
documentation or MMIO operations. Then, on that component you export functions that
will be needed by this new idle component.

This new idle component is only responsible for the idle interface... informative
and or control... sysfs comes here then.

Later the same shall happen with the freq stuff.  Freq should be exposed through
devfreq or hwmon... and for that we will have to create a new component. However
all the related low-level freq management will remain in the xe_guc_pc.

> 
> > 
> > > Br,
> > > Anshuman Gupta.
> > > > +
> > > > +	/*
> > > > +	 * Counter wrap handling
> > > > +	 * Store previous hw counter values for counter wrap-around
> > > > handling
> > > > +	 * Relying on sufficient frequency of queries otherwise counters can
> > > > still wrap.
> > > > +	 */
> > > > +	prev_residency = idle->prev_rc6_residency;
> > > > +	idle->prev_rc6_residency = cur_residency;
> > > > +
> > > > +	/* RC6 delta */
> > > > +	if (cur_residency >= prev_residency)
> > > > +		delta = cur_residency - prev_residency;
> > > > +	else
> > > > +		delta = cur_residency + (overflow_residency -
> > > > prev_residency);
> > > > +
> > > > +	/* Add delta to RC6 extended raw driver copy. */
> > > > +	cur_residency = idle->cur_rc6_residency + delta;
> > > > +	idle->cur_rc6_residency = cur_residency;
> > > > +
> > > > +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
> > > > }
> > > > +
> > > >   static ssize_t
> > > >   rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> > > > { @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
> > > > __ATTR(rc_status, 0444, rc_status_show, NULL);
> > > > 
> > > >   static ssize_t
> > > > -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char
> > > > *buf)
> > > > +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute
> > > > +*attr, char *buf)
> > > >   {
> > > >   	struct xe_idle *idle = kobj_to_idle(kobj);
> > > >   	struct xe_gt *gt = idle_to_gt(idle);
> > > > -	u32 reg;
> > > >   	ssize_t ret;
> > > > 
> > > >   	xe_device_mem_access_get(gt_to_xe(gt));
> > > > @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct
> > > > kobj_attribute *attr, char *buf)
> > > >   	if (ret)
> > > >   		goto out;
> > > > 
> > > > -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
> > > > -	ret = sysfs_emit(buf, "%u\n", reg);
> > > > +	ret = sysfs_emit(buf, "%llu\n",
> > > > +DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
> > > > 
> > > >   	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
> > > > XE_FORCEWAKE_ALL));
> > > >   out:
> > > > @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct
> > > > kobj_attribute *attr, char *buf)
> > > >   	return ret;
> > > >   }
> > > > 
> > > > -static const struct kobj_attribute rc6_residency = -__ATTR(rc6_residency,
> > > > 0444, rc6_residency_show, NULL);
> > > > +static const struct kobj_attribute rc6_residency_ms =
> > > > +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
> > > > 
> > > >   static const struct attribute *idle_attrs[] = {
> > > >   	&rc_status.attr,
> > > > -	&rc6_residency.attr,
> > > > +	&rc6_residency_ms.attr,
> > > >   	NULL,
> > > >   };
> > > > 
> > > > --
> > > > 2.40.0
> > > 

Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Riana Tauro]

On 5/5/2023 6:44 PM, Rodrigo Vivi wrote:
> On Fri, May 05, 2023 at 01:14:29PM +0530, Riana Tauro wrote:
>>
>>
>> On 5/4/2023 9:27 PM, Rodrigo Vivi wrote:
>>> On Thu, May 04, 2023 at 06:55:48AM -0400, Gupta, Anshuman wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Tauro, Riana <riana.tauro@intel.com>
>>>>> Sent: Wednesday, May 3, 2023 12:13 PM
>>>>> To: intel-xe@lists.freedesktop.org
>>>>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>>>>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
>>>>> Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
>>>>> <badal.nilawar@intel.com>
>>>>> Subject: [RFC PATCH 2/3] drm/xe : add rc6_residency in ms
>>>>>
>>>>> add rc6_residency in ms instead of rc6 residency counter.
>>>>> Handle wrap around for the counter
>>>>> The counter can still wrap as it relies on the frequency of counter being read
>>>>>
>>>>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++----
>>>>> ---
>>>>>    1 file changed, 37 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
>>>>> index 231bdb45a6b7..a7e97ddd7253 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_idle.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_idle.c
>>>>> @@ -11,6 +11,8 @@
>>>>>    #include "xe_idle.h"
>>>>>    #include "xe_mmio.h"
>>>>>
>>>>> +#define XE_RC6_MULTIPLIER      1280
>>>>> +
>>>>>    /*
>>>>>     * Render-C States:
>>>>>     * ================
>>>>> @@ -27,8 +29,7 @@
>>>>>     *
>>>>>     * device/gt#/gpu_idle/rc* *read-only* files:
>>>>>     * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
>>>>> - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
>>>>> - *		    Prone to overflows.
>>>>> + * - rc6_residency_ms: Provide the rc6_residency in ms
>>>>>     */
>>>>>
>>>>>    static struct xe_gt *idle_to_gt(struct xe_idle *idle) @@ -51,6 +52,35 @@
>>>>> static struct kobj_type xe_idle_kobj_type = {
>>>>>    	.sysfs_ops = &kobj_sysfs_ops,
>>>>>    };
>>>>>
>>>>> +static u64 rc6_residency_us(struct xe_idle *idle) {
>>>>> +	struct xe_gt *gt = idle_to_gt(idle);
>>>>> +	u64 cur_residency, delta, overflow_residency, prev_residency;
>>>>> +
>>>>> +	overflow_residency = BIT_ULL(32);
>>>>> +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
>>>> We shall use function pointer here to get the residency, so underlying function can read from
>>>> appropriate counter and  abstract it from sysfs show function, same comment for converting to
>>>> ms as well.
>>>
>>> Yes, please!
>>>
>>> Leave this new component as clean and free from the 'RC' stuff as possible.
>>> Leave all RC stuff inside xe_guc_pc and then this new infra is specific for
>>> the report and control so you can make this as generic as possible and likely
>>> extend to other stuff.
>>> And contain the platform stuff to the lower level.
>> Hi Rodrigo
>>
>> So do we have two entries for residency? rc6_residency under device/gt#/rc
>> and also displayed as idle_residency under device/gt#/gpuidle?
> 
> No, that was not what I meant. I'm sorry for not being clear.
> 
> We should have only one interface. Let's not duplicate stuff and minimize the interfaces.
> 
> What I tried to say was to leave all the RC6 stuff inside xe_guc_pc... All RC6 related
> documentation or MMIO operations. Then, on that component you export functions that
> will be needed by this new idle component.
> 
> This new idle component is only responsible for the idle interface... informative
> and or control... sysfs comes here then.
> 
> Later the same shall happen with the freq stuff.  Freq should be exposed through
> devfreq or hwmon... and for that we will have to create a new component. However
> all the related low-level freq management will remain in the xe_guc_pc.

Sorry for the misunderstanding Thanks for the clarification.
Will move the low level functions to guc_pc.

Thanks
Riana
> 
>>
>>>
>>>> Br,
>>>> Anshuman Gupta.
>>>>> +
>>>>> +	/*
>>>>> +	 * Counter wrap handling
>>>>> +	 * Store previous hw counter values for counter wrap-around
>>>>> handling
>>>>> +	 * Relying on sufficient frequency of queries otherwise counters can
>>>>> still wrap.
>>>>> +	 */
>>>>> +	prev_residency = idle->prev_rc6_residency;
>>>>> +	idle->prev_rc6_residency = cur_residency;
>>>>> +
>>>>> +	/* RC6 delta */
>>>>> +	if (cur_residency >= prev_residency)
>>>>> +		delta = cur_residency - prev_residency;
>>>>> +	else
>>>>> +		delta = cur_residency + (overflow_residency -
>>>>> prev_residency);
>>>>> +
>>>>> +	/* Add delta to RC6 extended raw driver copy. */
>>>>> +	cur_residency = idle->cur_rc6_residency + delta;
>>>>> +	idle->cur_rc6_residency = cur_residency;
>>>>> +
>>>>> +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
>>>>> }
>>>>> +
>>>>>    static ssize_t
>>>>>    rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>>>>> { @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
>>>>> __ATTR(rc_status, 0444, rc_status_show, NULL);
>>>>>
>>>>>    static ssize_t
>>>>> -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char
>>>>> *buf)
>>>>> +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute
>>>>> +*attr, char *buf)
>>>>>    {
>>>>>    	struct xe_idle *idle = kobj_to_idle(kobj);
>>>>>    	struct xe_gt *gt = idle_to_gt(idle);
>>>>> -	u32 reg;
>>>>>    	ssize_t ret;
>>>>>
>>>>>    	xe_device_mem_access_get(gt_to_xe(gt));
>>>>> @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct
>>>>> kobj_attribute *attr, char *buf)
>>>>>    	if (ret)
>>>>>    		goto out;
>>>>>
>>>>> -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
>>>>> -	ret = sysfs_emit(buf, "%u\n", reg);
>>>>> +	ret = sysfs_emit(buf, "%llu\n",
>>>>> +DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
>>>>>
>>>>>    	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
>>>>> XE_FORCEWAKE_ALL));
>>>>>    out:
>>>>> @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct
>>>>> kobj_attribute *attr, char *buf)
>>>>>    	return ret;
>>>>>    }
>>>>>
>>>>> -static const struct kobj_attribute rc6_residency = -__ATTR(rc6_residency,
>>>>> 0444, rc6_residency_show, NULL);
>>>>> +static const struct kobj_attribute rc6_residency_ms =
>>>>> +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
>>>>>
>>>>>    static const struct attribute *idle_attrs[] = {
>>>>>    	&rc_status.attr,
>>>>> -	&rc6_residency.attr,
>>>>> +	&rc6_residency_ms.attr,
>>>>>    	NULL,
>>>>>    };
>>>>>
>>>>> --
>>>>> 2.40.0
>>>>