[RFC PATCH 1/2] drm/xe: Implement APIs for measuring various events
Matthew Brost
matthew.brost at intel.com
Thu Jul 25 16:27:37 UTC 2024
On Thu, Jul 25, 2024 at 05:48:34PM +0200, Michal Wajdeczko wrote:
>
>
> On 25.07.2024 17:05, Nirmoy Das wrote:
> > Introduces a set of APIs for tracking and managing events.
> > This currently only supports counters.
> >
> > This can be conditionally enabled with the CONFIG_DRM_XE_STATS
> > config option.
> >
> > Cc: Matthew Brost <matthew.brost at intel.com>
> > Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
> > ---
> > drivers/gpu/drm/xe/Kconfig.debug | 11 +++
> > drivers/gpu/drm/xe/Makefile | 1 +
> > drivers/gpu/drm/xe/xe_stats.c | 145 +++++++++++++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_stats.h | 66 ++++++++++++++
> > 4 files changed, 223 insertions(+)
> > create mode 100644 drivers/gpu/drm/xe/xe_stats.c
> > create mode 100644 drivers/gpu/drm/xe/xe_stats.h
> >
> > diff --git a/drivers/gpu/drm/xe/Kconfig.debug b/drivers/gpu/drm/xe/Kconfig.debug
> > index bc177368af6c..4db6d707c1af 100644
> > --- a/drivers/gpu/drm/xe/Kconfig.debug
> > +++ b/drivers/gpu/drm/xe/Kconfig.debug
> > @@ -94,3 +94,14 @@ config DRM_XE_USERPTR_INVAL_INJECT
> >
> > Recomended for driver developers only.
> > If in doubt, say "N".
> > +
> > +config DRM_XE_STATS
> > + bool "Enable XE statistics"
> > + default n
> > + help
> > + Choose this option to enable support for collecting and
> > + displaying XE statistics as debugfs.
> > +
> > + Recommended for driver developers only.
> > +
> > + If in doubt, say "N".
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index 1ff9602a52f6..c5d9ec22c6bf 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -117,6 +117,7 @@ xe-y += xe_bb.o \
> > xe_wopcm.o
> >
> > xe-$(CONFIG_HMM_MIRROR) += xe_hmm.o
> > +xe-$(CONFIG_DRM_XE_STATS) += xe_stats.o
> >
> > # graphics hardware monitoring (HWMON) support
> > xe-$(CONFIG_HWMON) += xe_hwmon.o
> > diff --git a/drivers/gpu/drm/xe/xe_stats.c b/drivers/gpu/drm/xe/xe_stats.c
> > new file mode 100644
> > index 000000000000..1b8910900fd3
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_stats.c
> > @@ -0,0 +1,145 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#include <linux/debugfs.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/slab.h>
> > +
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_managed.h>
> > +#include <drm/drm_print.h>
> > +
> > +#include "xe_stats.h"
> > +
> > +#ifdef CONFIG_DRM_XE_STATS
> > +static struct xe_stats_entry *find_stats_entry(struct xe_stats *stats, const char *name)
> > +{
> > + struct xe_stats_entry *stats_entry;
> > +
> > + list_for_each_entry(stats_entry, &stats->entries, list) {
> > + if (strcmp(stats_entry->name, name) == 0)
> > + return stats_entry;
>
> hmm, this seems to be very costly and likely will not scale well once we
> would like to add more counters
>
Agree this won't scale well. See my suggestion a reply to Nirmoy. Let me
know what you think.
Matt
> can't we just use CONFIG_DRM_XE_STATS to hide different stats structures
> provide nop variants of the helpers that would update them ?
>
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int stats_show(struct seq_file *m, void *v)
> > +{
> > + struct xe_stats_entry *entry = m->private;
> > +
> > + if (entry && !entry->enabled)
> > + return 0;
> > +
> > + switch (entry->type) {
> > + case XE_STATS_TYPE_COUNTER:
> > + seq_printf(m, "%s: Counter value: %llu\n", entry->name,
> > + atomic64_read(&entry->data.counter.value));
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int stats_open(struct inode *inode, struct file *file)
> > +{
> > + return single_open(file, stats_show, inode->i_private);
> > +}
> > +
> > +static const struct file_operations stats_fops = {
> > + .owner = THIS_MODULE,
> > + .open = stats_open,
> > + .read = seq_read,
> > + .release = single_release,
> > +};
> > +
> > +struct xe_stats *xe_stats_init(struct drm_device *drm, struct dentry *parent)
> > +{
> > + struct xe_stats *stats;
> > +
> > + stats = drmm_kzalloc(drm, sizeof(*stats), GFP_KERNEL);
> > + if (!stats) {
> > + drm_err(drm, "Failed to allocate memory for xe_stats\n");
> > + return NULL;
> > + }
> > +
> > + stats->drm = drm;
> > + INIT_LIST_HEAD(&stats->entries);
> > +
> > + stats->debugfs_entry = debugfs_create_dir("stats", parent);
> > + if (!stats->debugfs_entry) {
> > + drm_err(drm, "Failed to create stats debugfs directory\n");
> > + return NULL;
> > + }
> > +
> > + return stats;
> > +}
> > +
> > +void xe_stats_add_entry(struct xe_stats *stats, const char *name, enum xe_stats_type type)
> > +{
> > + struct xe_stats_entry *entry;
> > + struct dentry *debugfs_entry;
> > +
> > + if (!stats || !name)
> > + return;
> > +
> > + entry = drmm_kzalloc(stats->drm, sizeof(*entry), GFP_KERNEL);
> > + if (!entry)
> > + return;
> > +
> > + entry->name = drmm_kstrdup(stats->drm, name, GFP_KERNEL);
> > + if (!entry->name)
> > + return;
> > +
> > + entry->type = type;
> > + entry->enabled = true;
> > +
> > + switch (type) {
> > + case XE_STATS_TYPE_COUNTER:
> > + atomic64_set(&entry->data.counter.value, 0);
> > + break;
> > + default:
> > + return;
> > + }
> > +
> > + list_add(&entry->list, &stats->entries);
> > +
> > + debugfs_entry = debugfs_create_file(name, 0444, stats->debugfs_entry, entry, &stats_fops);
> > + if (!debugfs_entry) {
> > + drm_err(stats->drm, "Failed to create stats debugfs file for %s\n", name);
> > + list_del(&entry->list);
> > + return;
> > + }
> > +
> > + entry->dentry = debugfs_entry;
> > +}
> > +
> > +void xe_stats_increment_counter(struct xe_stats *stats, const char *name)
> > +{
> > + struct xe_stats_entry *entry;
> > +
> > + if (!stats || !name)
> > + return;
> > +
> > + entry = find_stats_entry(stats, name);
> > + if (entry && entry->type == XE_STATS_TYPE_COUNTER && entry->enabled)
> > + atomic64_inc(&entry->data.counter.value);
> > +}
> > +
> > +void xe_stats_decrement_counter(struct xe_stats *stats, const char *name)
> > +{
> > + struct xe_stats_entry *entry;
> > +
> > + if (!stats || !name)
> > + return;
> > +
> > + entry = find_stats_entry(stats, name);
> > + if (entry && entry->type == XE_STATS_TYPE_COUNTER && entry->enabled)
> > + atomic64_dec(&entry->data.counter.value);
> > +}
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/xe/xe_stats.h b/drivers/gpu/drm/xe/xe_stats.h
> > new file mode 100644
> > index 000000000000..dba79ae28714
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_stats.h
> > @@ -0,0 +1,66 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +#ifndef _XE_STATS_H_
> > +#define _XE_STATS_H_
> > +
> > +#include <linux/atomic.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/list.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> > +
> > +#include <drm/drm_print.h>
> > +
> > +struct drm_device;
> > +
> > +enum xe_stats_type {
> > + XE_STATS_TYPE_COUNTER,
> > +};
> > +
> > +#ifdef CONFIG_DRM_XE_STATS
> > +struct xe_stats_entry {
> > + const char *name;
> > + enum xe_stats_type type;
> > + bool enabled;
> > + union {
> > + struct {
> > + atomic64_t value;
> > + } counter;
> > + } data;
> > + struct list_head list;
> > + struct dentry *dentry;
> > +};
> > +
> > +struct xe_stats {
> > + struct dentry *debugfs_entry;
> > + struct list_head entries;
> > + struct drm_device *drm;
> > +};
> > +
> > +struct xe_stats *xe_stats_init(struct drm_device *drm, struct dentry *parent);
> > +void xe_stats_add_entry(struct xe_stats *stats, const char *name, enum xe_stats_type type);
> > +void xe_stats_increment_counter(struct xe_stats *c, const char *name);
> > +void xe_stats_decrement_counter(struct xe_stats *stats, const char *name);
> > +
> > +#else /* CONFIG_DRM_XE_STATS not defined */
> > +struct xe_stats;
> > +
> > +static inline struct xe_stats *
> > +xe_stats_init(struct drm_device *drm, struct dentry *parent)
> > +{
> > + return NULL;
> > +}
> > +
> > +static inline void
> > +xe_stats_add_entry(struct xe_stats *stats, const char *name, enum xe_stats_type type) { }
> > +static inline void
> > +xe_stats_increment_counter(struct xe_stats *stats, const char *name) { }
> > +static inline void
> > +xe_stats_decrement_counter(struct xe_stats *stats, const char *name) { }
> > +
> > +#endif /* CONFIG_DRM_XE_STATS */
> > +
> > +#endif /* _XE_STATS_H_ */
> > +
More information about the Intel-xe
mailing list