[PATCH 1/2] drm/xe/gt: Add APIs for printing stats over debugfs

[Matthew Brost]

On Tue, Aug 06, 2024 at 09:45:03PM +0200, Nirmoy Das wrote:
> Add skeleton APIs for recording and printing various stats over
> debugfs. This currently only added counter types stats which is backed
> by atomic_t and wrapped with CONFIG_DRM_XE_STATS so this can be disabled
> on production system.
> 

Yes, kernel doc which you pointed out.

> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko 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        |  2 ++
>  drivers/gpu/drm/xe/xe_gt_debugfs.c |  4 ++++
>  drivers/gpu/drm/xe/xe_gt_stats.c   | 30 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_stats.h   | 31 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_types.h   |  7 +++++++
>  6 files changed, 85 insertions(+)
>  create mode 100644 drivers/gpu/drm/xe/xe_gt_stats.c
>  create mode 100644 drivers/gpu/drm/xe/xe_gt_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..b0bbb248f644 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -143,6 +143,8 @@ xe-$(CONFIG_PCI_IOV) += \
>  	xe_pci_sriov.o \
>  	xe_sriov_pf.o
>  
> +xe-$(CONFIG_DRM_XE_STATS) += xe_gt_stats.o
> +
>  # include helpers for tests even when XE is built-in
>  ifdef CONFIG_DRM_XE_KUNIT_TEST
>  xe-y += tests/xe_kunit_helpers.o
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index 5e7fd937917a..3765c04919dc 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -17,6 +17,7 @@
>  #include "xe_gt_mcr.h"
>  #include "xe_gt_sriov_pf_debugfs.h"
>  #include "xe_gt_sriov_vf_debugfs.h"
> +#include "xe_gt_stats.h"
>  #include "xe_gt_topology.h"
>  #include "xe_hw_engine.h"
>  #include "xe_lrc.h"
> @@ -286,6 +287,9 @@ static const struct drm_info_list debugfs_list[] = {
>  	{"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data = bcs_default_lrc},
>  	{"default_lrc_vcs", .show = xe_gt_debugfs_simple_show, .data = vcs_default_lrc},
>  	{"default_lrc_vecs", .show = xe_gt_debugfs_simple_show, .data = vecs_default_lrc},
> +#if IS_ENABLED(CONFIG_DRM_XE_STATS)
> +	{"stats", .show = xe_gt_debugfs_simple_show, .data = xe_gt_stats_print_info},
> +#endif
>  };
>  
>  void xe_gt_debugfs_register(struct xe_gt *gt)
> diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c
> new file mode 100644
> index 000000000000..cc69d748e8f6
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_gt_stats.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include <linux/atomic.h>
> +
> +#include "xe_gt.h"
> +#include "xe_gt_stats.h"
> +
> +#ifdef CONFIG_DRM_XE_STATS
> +void xe_gt_stats_incr(struct xe_gt *gt, enum xe_gt_counter_stats id, int incr)
> +{
> +	atomic_add(incr, &gt->stats.counters[id]);
> +}
> +
> +static const char *const stat_description[] = {
> +};
> +
> +int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p)
> +{
> +	enum xe_gt_counter_stats id;
> +
> +	for (id = 0; id < XE_GT_COUNTER_STATS_NUM; ++id)
> +		drm_printf(p, "%s: %d\n", stat_description[id],
> +			   atomic_read(&gt->stats.counters[id]));
> +
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/gpu/drm/xe/xe_gt_stats.h b/drivers/gpu/drm/xe/xe_gt_stats.h
> new file mode 100644
> index 000000000000..e83155ab1fdb
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_gt_stats.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#ifndef _XE_GT_STATS_H_
> +#define _XE_GT_STATS_H_
> +
> +#include <drm/drm_print.h>
> +
> +struct xe_gt;
> +//struct drm_printer;
> +
> +enum xe_gt_counter_stats {
> +	XE_GT_COUNTER_STATS_NUM,
> +};
> +
> +#ifdef CONFIG_DRM_XE_STATS
> +int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p);
> +void xe_gt_stats_incr(struct xe_gt *gt, enum xe_gt_counter_stats stat, int incr);
> +#else
> +int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p)

static inline int xe_gt_stats_print_info(...

This will fix CI too.

But overall LGTM and pretty much what I had in mind. We can device level
stats too with this series being a template.

Matt
 
> +{
> +}
> +
> +void xe_gt_stats_incr(struct xe_gt *gt, enum xe_gt_counter_stats stat, int incr)
> +{
> +}
> +
> +#endif /* CONFIG_DRM_XE_STATS */
> +#endif /* _XE_GT_STATS_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index 631928258d71..0ded57b56628 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -10,6 +10,7 @@
>  #include "xe_gt_idle_types.h"
>  #include "xe_gt_sriov_pf_types.h"
>  #include "xe_gt_sriov_vf_types.h"
> +#include "xe_gt_stats.h"
>  #include "xe_hw_engine_types.h"
>  #include "xe_hw_fence_types.h"
>  #include "xe_oa.h"
> @@ -133,6 +134,12 @@ struct xe_gt {
>  		u8 has_indirect_ring_state:1;
>  	} info;
>  
> +#if IS_ENABLED(CONFIG_DRM_XE_STATS)
> +	struct {
> +		atomic_t counters[XE_GT_COUNTER_STATS_NUM];
> +	} stats;
> +#endif
> +
>  	/**
>  	 * @mmio: mmio info for GT.  All GTs within a tile share the same
>  	 * register space, but have their own copy of GSI registers at a
> -- 
> 2.42.0
>