[PATCH v3 1/2] drm/xe/gt: Add APIs for printing stats over debugfs
Nirmoy Das
nirmoy.das at intel.com
Fri Aug 9 08:37:43 UTC 2024
On 8/9/2024 6:35 AM, Matthew Brost wrote:
> On Thu, Aug 08, 2024 at 11:19:14AM +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.
>>
>> v3: s/CONFIG_DRM_XE_STATS/CONFIG_DEBUG_FS(Lucas)
>> v2: add missing docs
>> Add boundary checks for stats id and other improvements (Michal)
>> Fix build when CONFIG_DRM_XE_STATS is disabled(Matt)
>>
> This will need to be rebased on Lucas's series [1].
>
> [1] https://patchwork.freedesktop.org/series/137053/
>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Cc: Matthew Brost <matthew.brost at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Sai Gowtham Ch <sai.gowtham.ch at intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile | 2 ++
>> drivers/gpu/drm/xe/xe_gt_debugfs.c | 2 ++
>> drivers/gpu/drm/xe/xe_gt_stats.c | 48 ++++++++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_gt_stats.h | 32 ++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_gt_types.h | 9 ++++++
>> 5 files changed, 93 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/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 1ff9602a52f6..390e5652cd89 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -287,6 +287,8 @@ ifeq ($(CONFIG_DEBUG_FS),y)
>> i915-display/intel_pipe_crc.o
>> endif
>>
>> +xe-$(CONFIG_DEBUG_FS) += xe_gt_stats.o
>> +
>> obj-$(CONFIG_DRM_XE) += xe.o
>> obj-$(CONFIG_DRM_XE_KUNIT_TEST) += tests/
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
>> index 5e7fd937917a..5125d76ccfac 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,7 @@ 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},
>> + {"stats", .show = xe_gt_debugfs_simple_show, .data = xe_gt_stats_print_info},
>> };
>>
>> 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..74ed9342af91
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_gt_stats.c
>> @@ -0,0 +1,48 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#include <linux/atomic.h>
>> +
>> +#include <drm/drm_print.h>
>> +
>> +#include "xe_gt.h"
>> +#include "xe_gt_stats.h"
>> +
>> +/**
>> + * xe_gt_stats_incr - Increments the specified stats counter
>> + * @gt: graphics tile
>> + * @id: xe_gt_stats_id type id that needs to be incremented
>> + * @incr: value to be incremented with
>> + *
>> + * Increments the specified stats counter.
>> + */
>> +void xe_gt_stats_incr(struct xe_gt *gt, enum xe_gt_stats_id id, int incr)
>> +{
>> + if (id >= __XE_GT_STATS_NUM_IDS)
>> + return;
>> +
>> + atomic_add(incr, >->stats.counters[id]);
>> +}
>> +
>> +static const char *const stat_description[__XE_GT_STATS_NUM_IDS] = {
>> +};
>> +
>> +/**
>> + * xe_gt_stats_print_info - Print the GT stats
>> + * @gt: graphics tile
>> + * @p: drm_printer where it will be printed out.
>> + *
>> + * This prints out all the available GT stats.
>> + */
>> +int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p)
>> +{
>> + enum xe_gt_stats_id id;
>> +
>> + for (id = 0; id < __XE_GT_STATS_NUM_IDS; ++id)
>> + drm_printf(p, "%s: %d\n", stat_description[id],
>> + atomic_read(>->stats.counters[id]));
>> +
>> + return 0;
>> +}
>> 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..b97363e99ba7
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_gt_stats.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_GT_STATS_H_
>> +#define _XE_GT_STATS_H_
>> +
>> +struct xe_gt;
>> +struct drm_printer;
>> +
>> +enum xe_gt_stats_id {
>> + /* must be the last entry */
>> + __XE_GT_STATS_NUM_IDS,
>> +};
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +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_stats_id id, int incr);
> I think Lucas wants this to be s/enum xe_gt_stats_id id/const enum xe_gt_stats_id id/
I will make it const.
I think Lucas also wanted to add a build time check for id but that
turns out complicated as I have to move this func to
the header file as static inline. Accessing gt members in a header file
doesn't work without adding xe_gt.h that again
ends with recursive include. So decided to keep this as v2
>
>> +#else
>> +static inline int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p)
>> +{
>> + return 0;
>> +}
> You don't need this as the only caller will compile out without
> CONFIG_DEBUG_FS after Lucas's series.
>
> With the nits fixed:
> Reviewed-by: Matthew Brost <matthew.brost at intel.com>
Thanks, I will rebase and resnd.
Nirmoy
>
>> +
>> +static inline void xe_gt_stats_incr(struct xe_gt *gt, enum xe_gt_stats_id id,
>> + int incr)
>> +{
>> +}
>> +
>> +#endif
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>> index 631928258d71..8e46c0969bc7 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,14 @@ struct xe_gt {
>> u8 has_indirect_ring_state:1;
>> } info;
>>
>> +#if IS_ENABLED(CONFIG_DEBUG_FS)
>> + /** @stats: GT stats */
>> + struct {
>> + /** @stats.counters: counters for various GT stats */
>> + atomic_t counters[__XE_GT_STATS_NUM_IDS];
>> + } 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
>>
More information about the Intel-xe
mailing list