On Wed, Jan 22, 2025 at 04:21:55PM +0530, Riana Tauro wrote:
>Hi Lucas
>
>On 1/22/2025 11:53 AM, Lucas De Marchi wrote:
>>From: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>
>>Basic PMU enabling patch. Setup the basic framework
>>for adding events/timers. This patch was previously
>>reviewed here -
>>https://patchwork.freedesktop.org/series/119504/
>>
>>Based on previous versions by Bommu Krishnaiah, Aravind Iddamsetty and
>>Riana Tauro, using i915 and rapl as reference implementation.
>>
>>Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>---
>> drivers/gpu/drm/xe/Makefile | 2 +
>> drivers/gpu/drm/xe/xe_device.c | 3 +
>> drivers/gpu/drm/xe/xe_device_types.h | 4 +
>> drivers/gpu/drm/xe/xe_pmu.c | 299 +++++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_pmu.h | 20 ++
>> drivers/gpu/drm/xe/xe_pmu_types.h | 43 ++++
>> 6 files changed, 371 insertions(+)
>> create mode 100644 drivers/gpu/drm/xe/xe_pmu.c
>> create mode 100644 drivers/gpu/drm/xe/xe_pmu.h
>> create mode 100644 drivers/gpu/drm/xe/xe_pmu_types.h
>>
>>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>>index 68861db5f27ce..aa0d981663e4c 100644
>>--- a/drivers/gpu/drm/xe/Makefile
>>+++ b/drivers/gpu/drm/xe/Makefile
>>@@ -305,6 +305,8 @@ endif
>> xe-$(CONFIG_DRM_XE_DP_TUNNEL) += \
>> i915-display/intel_dp_tunnel.o
>>+xe-$(CONFIG_PERF_EVENTS) += xe_pmu.o
>>+
>> obj-$(CONFIG_DRM_XE) += xe.o
>> obj-$(CONFIG_DRM_XE_KUNIT_TEST) += tests/
>>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>index bd6191e1ed3e7..f3f754beb812b 100644
>>--- a/drivers/gpu/drm/xe/xe_device.c
>>+++ b/drivers/gpu/drm/xe/xe_device.c
>>@@ -49,6 +49,7 @@
>> #include "xe_pat.h"
>> #include "xe_pcode.h"
>> #include "xe_pm.h"
>>+#include "xe_pmu.h"
>> #include "xe_query.h"
>> #include "xe_sriov.h"
>> #include "xe_tile.h"
>>@@ -871,6 +872,8 @@ int xe_device_probe(struct xe_device *xe)
>> xe_oa_register(xe);
>>+ xe_pmu_register(&xe->pmu);
>>+
>> xe_debugfs_register(xe);
>> xe_hwmon_register(xe);
>>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>>index 16ebb2859877f..58e79e19deaad 100644
>>--- a/drivers/gpu/drm/xe/xe_device_types.h
>>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>>@@ -18,6 +18,7 @@
>> #include "xe_memirq_types.h"
>> #include "xe_oa_types.h"
>> #include "xe_platform_types.h"
>>+#include "xe_pmu_types.h"
>> #include "xe_pt_types.h"
>> #include "xe_sriov_types.h"
>> #include "xe_step_types.h"
>>@@ -514,6 +515,9 @@ struct xe_device {
>> int mode;
>> } wedged;
>>+ /** @pmu: performance monitoring unit */
>>+ struct xe_pmu pmu;
>>+
>> #ifdef TEST_VM_OPS_ERROR
>> /**
>> * @vm_inject_error_position: inject errors at different places in VM
>>diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c
>>new file mode 100644
>>index 0000000000000..c5641af6e9a91
>>--- /dev/null
>>+++ b/drivers/gpu/drm/xe/xe_pmu.c
>>@@ -0,0 +1,299 @@
>>+// SPDX-License-Identifier: MIT
>>+/*
>>+ * Copyright © 2025 Intel Corporation
>>+ */
>>+
>
>Some of these headers are unused
>>+#include <drm/drm_drv.h>
>>+#include <drm/drm_managed.h> +#include <drm/xe_drm.h>
>above 2 unused
>>+
>>+#include "regs/xe_gt_regs.h"
>unused
>>+#include "xe_device.h"
>
>>+#include "xe_force_wake.h"
>>+#include "xe_gt_clock.h"
>>+#include "xe_gt_printk.h"
>>+#include "xe_mmio.h"
>>+#include "xe_macros.h"
>unused
>>+#include "xe_pm.h"
>>+#include "xe_pmu.h"
>>+
>>+/**
>>+ * DOC: Xe PMU (Performance Monitoring Unit)
>>+ *
>>+ * Expose events/counters like GT-C6 residency and GT frequency to user land.
>>+ * Events are per device. The GT can be selected with an extra config sub-field
>>+ * (bits 60-63).
>>+ *
>>+ * All events are listed in sysfs:
>>+ *
>>+ * $ ls -ld /sys/bus/event_source/devices/xe_*
>>+ * $ ls /sys/bus/event_source/devices/xe_0000_00_02.0/events/
>>+ * $ ls /sys/bus/event_source/devices/xe_0000_00_02.0/format/
>>+ *
>>+ * The format directory has info regarding the configs that can be used.
>>+ * The standard perf tool can be used to grep for a certain event as well.
>>+ * Example:
>>+ *
>>+ * $ perf list | grep gt-c6
>>+ *
>>+ * To sample a specific event for a GT at regular intervals:
>>+ *
>>+ * $ perf stat -e <event_name,gt=> -I <interval>
>>+ */
>>+
>>+#define XE_PMU_EVENT_GT_MASK GENMASK_ULL(63, 60)
>>+#define XE_PMU_EVENT_ID_MASK GENMASK_ULL(11, 0)
>>+
>>+static unsigned int config_to_event_id(u64 config)
>>+{
>>+ return FIELD_GET(XE_PMU_EVENT_ID_MASK, config);
>>+}
>>+
>>+static unsigned int config_to_gt_id(u64 config)
>>+{
>>+ return FIELD_GET(XE_PMU_EVENT_GT_MASK, config);
>>+}
>>+
>>+static struct xe_gt *event_to_gt(struct perf_event *event)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+ u64 gt = config_to_gt_id(event->attr.config);
>>+
>>+ return xe_device_get_gt(xe, gt);
>>+}
>>+
>>+static bool event_supported(struct xe_pmu *pmu, unsigned int gt,
>>+ unsigned int id)
>>+{
>>+ if (gt >= XE_MAX_GT_PER_TILE)
>>+ return false;
>>+
>>+ return false;
>>+}
>>+
>>+static void xe_pmu_event_destroy(struct perf_event *event)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+
>>+ drm_WARN_ON(&xe->drm, event->parent);
>>+ drm_dev_put(&xe->drm);
>>+}
>>+
>>+static int xe_pmu_event_init(struct perf_event *event)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+ struct xe_pmu *pmu = &xe->pmu;
>>+ unsigned int id, gt;
>>+
>>+ if (!pmu->registered)
>>+ return -ENODEV;
>>+
>>+ if (event->attr.type != event->pmu->type)
>>+ return -ENOENT;
>>+
>>+ /* unsupported modes and filters */
>>+ if (event->attr.sample_period) /* no sampling */
>>+ return -EINVAL;
>>+
>>+ if (event->cpu < 0)
>>+ return -EINVAL;
>>+
>>+ gt = config_to_gt_id(event->attr.config);
>>+ id = config_to_event_id(event->attr.config);
>>+ if (!event_supported(pmu, gt, id))
>>+ return -ENOENT;
>>+
>>+ if (has_branch_stack(event))
>>+ return -EOPNOTSUPP;
>>+
>>+ if (!event->parent) {
>>+ drm_dev_get(&xe->drm);
>>+ event->destroy = xe_pmu_event_destroy;
>>+ }
>>+
>>+ return 0;
>>+}
>>+
>>+static u64 __xe_pmu_event_read(struct perf_event *event)
>>+{
>>+ struct xe_gt *gt = event_to_gt(event);
>>+ u64 val = 0;
>>+
>>+ if (!gt)
>>+ return 0;
>>+
>>+ return val;
>>+}
>>+
>>+static void xe_pmu_event_read(struct perf_event *event)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+ struct hw_perf_event *hwc = &event->hw;
>>+ struct xe_pmu *pmu = &xe->pmu;
>>+ u64 prev, new;
>>+
>>+ if (!pmu->registered) {
>>+ event->hw.state = PERF_HES_STOPPED;
>>+ return;
>>+ }
>>+
>>+ prev = local64_read(&hwc->prev_count);
>>+ do {
>>+ new = __xe_pmu_event_read(event);
>>+ } while (!local64_try_cmpxchg(&hwc->prev_count, &prev, new));
>>+
>>+ local64_add(new - prev, &event->count);
>>+}
>>+
>>+static void xe_pmu_enable(struct perf_event *event)
>>+{
>>+ /*
>>+ * Store the current counter value so we can report the correct delta
>>+ * for all listeners. Even when the event was already enabled and has
>>+ * an existing non-zero value.
>>+ */
>>+ local64_set(&event->hw.prev_count, __xe_pmu_event_read(event));
>>+}
>>+
>>+static void xe_pmu_event_start(struct perf_event *event, int flags)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+ struct xe_pmu *pmu = &xe->pmu;
>>+
>>+ if (!pmu->registered)
>>+ return;
>>+
>>+ xe_pmu_enable(event);
>>+ event->hw.state = 0;
>>+}
>>+
>>+static void xe_pmu_event_stop(struct perf_event *event, int flags)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+ struct xe_pmu *pmu = &xe->pmu;
>>+
>>+ if (pmu->registered)
>>+ if (flags & PERF_EF_UPDATE)
>>+ xe_pmu_event_read(event);
>>+
>>+ event->hw.state = PERF_HES_STOPPED;
>>+}
>>+
>>+static int xe_pmu_event_add(struct perf_event *event, int flags)
>>+{
>>+ struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
>>+ struct xe_pmu *pmu = &xe->pmu;
>>+
>>+ if (!pmu->registered)
>>+ return -ENODEV;
>>+
>>+ if (flags & PERF_EF_START)
>>+ xe_pmu_event_start(event, flags);
>>+
>>+ return 0;
>>+}
>>+
>>+static void xe_pmu_event_del(struct perf_event *event, int flags)
>>+{
>>+ xe_pmu_event_stop(event, PERF_EF_UPDATE);
>>+}
>>+
>>+PMU_FORMAT_ATTR(gt, "config:60-63");
>>+PMU_FORMAT_ATTR(event, "config:0-11");
>>+
>>+static struct attribute *pmu_format_attrs[] = {
>>+ &format_attr_event.attr,
>>+ &format_attr_gt.attr,
>>+ NULL,
>>+};
>>+
>>+static const struct attribute_group pmu_format_attr_group = {
>>+ .name = "format",
>>+ .attrs = pmu_format_attrs,
>>+};
>>+
>>+static struct attribute *pmu_event_attrs[] = {
>>+ /* No events yet */
>>+ NULL,
>>+};
>>+
>>+static const struct attribute_group pmu_events_attr_group = {
>>+ .name = "events",
>>+ .attrs = pmu_event_attrs,
>>+};
>>+
>>+/**
>>+ * xe_pmu_unregister() - Remove/cleanup PMU registration
>>+ * @arg: Ptr to pmu
>>+ */
>>+static void xe_pmu_unregister(void *arg)
>>+{
>>+ struct xe_pmu *pmu = arg;
>>+ struct xe_device *xe = container_of(pmu, typeof(*xe), pmu);
>>+
>>+ if (!pmu->registered)
>>+ return;
>>+
>>+ pmu->registered = false;
>>+
>>+ perf_pmu_unregister(&pmu->base);
>>+ kfree(pmu->name);
>>+}
>>+
>>+/**
>>+ * xe_pmu_register() - Define basic PMU properties for Xe and add event callbacks.
>>+ * @pmu: the PMU object
>>+ *
>>+ * Returns 0 on success and an appropriate error code otherwise
>>+ */
>>+int xe_pmu_register(struct xe_pmu *pmu)
>>+{
>>+ struct xe_device *xe = container_of(pmu, typeof(*xe), pmu);
>>+ static const struct attribute_group *attr_groups[] = {
>>+ &pmu_format_attr_group,
>>+ &pmu_events_attr_group,
>>+ NULL
>>+ };
>>+ int ret = -ENOMEM;
>>+ char *name;
>>+
>>+ if (IS_SRIOV_VF(xe))
>>+ return 0;
>>+
>>+ raw_spin_lock_init(&pmu->lock);
>>+
>>+ name = kasprintf(GFP_KERNEL, "xe_%s",
>>+ dev_name(xe->drm.dev));
>>+ if (!name)
>>+ goto err;
>>+
>>+ /* tools/perf reserves colons as special. */
>>+ strreplace(name, ':', '_');
>>+
>>+ pmu->name = name;
>>+ pmu->base.attr_groups = attr_groups;
>>+ pmu->base.scope = PERF_PMU_SCOPE_SYS_WIDE;
>>+ pmu->base.module = THIS_MODULE;
>>+ pmu->base.task_ctx_nr = perf_invalid_context;
>>+ pmu->base.event_init = xe_pmu_event_init;
>>+ pmu->base.add = xe_pmu_event_add;
>>+ pmu->base.del = xe_pmu_event_del;
>>+ pmu->base.start = xe_pmu_event_start;
>>+ pmu->base.stop = xe_pmu_event_stop;
>>+ pmu->base.read = xe_pmu_event_read;
>>+
>>+ ret = perf_pmu_register(&pmu->base, pmu->name, -1);
>>+ if (ret)
>>+ goto err_name;
>>+
>>+ pmu->registered = true;
>>+
>>+ return devm_add_action_or_reset(xe->drm.dev, xe_pmu_unregister, pmu);
>>+
>>+err_name:
>>+ kfree(name);
>>+err:
>>+ drm_err(&xe->drm, "Failed to register PMU (ret=%d)!\n", ret);
>>+
>>+ return ret;
>>+}
>>diff --git a/drivers/gpu/drm/xe/xe_pmu.h b/drivers/gpu/drm/xe/xe_pmu.h
>>new file mode 100644
>>index 0000000000000..f9dfe77d00cb6
>>--- /dev/null
>>+++ b/drivers/gpu/drm/xe/xe_pmu.h
>>@@ -0,0 +1,20 @@
>>+/* SPDX-License-Identifier: MIT */
>>+/*
>>+ * Copyright © 2025 Intel Corporation
>>+ */
>>+
>>+#ifndef _XE_PMU_H_
>>+#define _XE_PMU_H_
>>+
>>+#include "xe_pmu_types.h"
>>+
>>+struct xe_gt;
>unused
>>+
>>+#if IS_ENABLED(CONFIG_PERF_EVENTS)
>>+int xe_pmu_register(struct xe_pmu *pmu);
>>+#else
>>+static inline void xe_pmu_register(struct xe_pmu *pmu) {}
this also missed the update to return int.
>>+#endif
>>+
>>+#endif
>>+
>>diff --git a/drivers/gpu/drm/xe/xe_pmu_types.h b/drivers/gpu/drm/xe/xe_pmu_types.h
>>new file mode 100644
>>index 0000000000000..e0cf7169f4fda
>>--- /dev/null
>>+++ b/drivers/gpu/drm/xe/xe_pmu_types.h
>>@@ -0,0 +1,43 @@
>>+/* SPDX-License-Identifier: MIT */
>>+/*
>>+ * Copyright © 2025 Intel Corporation
>>+ */
>>+
>>+#ifndef _XE_PMU_TYPES_H_
>>+#define _XE_PMU_TYPES_H_
>>+
>>+#include <linux/perf_event.h>
>>+#include <linux/spinlock_types.h>
>>+
>>+enum {
>>+ __XE_NUM_PMU_SAMPLERS
>>+};
>This enum is not used. can be removed
Thanks. After all the cleanups and patches re-split some of the
headers/struct don't make sense anymore in this initial patch. I
cleaned them and have things lined up for next version.
thanks
Lucas De Marchi
>
>Thanks,
>Riana
>>+
>>+#define XE_PMU_MAX_GT 2
>>+
>>+/**
>>+ * struct xe_pmu - PMU related data per Xe device
>>+ *
>>+ * Stores per device PMU info that includes event/perf attributes and
>>+ * sampling counters across all GTs for this device.
>>+ */
>>+struct xe_pmu {
>>+ /**
>>+ * @base: PMU base.
>>+ */
>>+ struct pmu base;
>>+ /**
>>+ * @registered: PMU is registered and not in the unregistering process.
>>+ */
>>+ bool registered;
>>+ /**
>>+ * @name: Name as registered with perf core.
>>+ */
>>+ const char *name;
>>+ /**
>>+ * @lock: Lock protecting enable mask and ref count handling.
>>+ */
>>+ raw_spinlock_t lock;
>>+};
>>+
>>+#endif
>