Re: [PATCH 2/2] drm/xe: Create debugfs for tlb inval stats

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 23.07.2024 13:16, Nirmoy Das wrote:
> Create debugfs file for each GT to dump tlb sent/receive
> stats.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_debugfs.c          | 9 +++++++++
>  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 7 +++++++
>  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h | 2 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index 5e7fd937917a..959d979927dd 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_tlb_invalidation.h"
>  #include "xe_gt_topology.h"
>  #include "xe_hw_engine.h"
>  #include "xe_lrc.h"
> @@ -269,6 +270,13 @@ static int vecs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
>  	return 0;
>  }
>  
> +static int tlb_stats(struct xe_gt *gt, struct drm_printer *p)
> +{
> +	xe_gt_tlb_dump(gt, p);
> +
> +	return 0;
> +}
> +
>  static const struct drm_info_list debugfs_list[] = {
>  	{"hw_engines", .show = xe_gt_debugfs_simple_show, .data = hw_engines},
>  	{"force_reset", .show = xe_gt_debugfs_simple_show, .data = force_reset},
> @@ -286,6 +294,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},
> +	{"tlb_stats", .show = xe_gt_debugfs_simple_show, .data = tlb_stats},
>  };
>  
>  void xe_gt_debugfs_register(struct xe_gt *gt)
> diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> index f84717c1aafa..62a6f42b6c60 100644
> --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> @@ -540,3 +540,10 @@ void xe_gt_tlb_invalidation_fence_fini(struct xe_gt_tlb_invalidation_fence *fenc
>  {
>  	xe_pm_runtime_put(gt_to_xe(fence->gt));
>  }
> +
> +void xe_gt_tlb_dump(struct xe_gt *gt, struct drm_printer *p)

if you make this function as returning "int" then you will be able to
plug it directly into debugfs_list without the tlb_stats() wrapper

> +{
> +	drm_printf(p, "GT%d, TLB Requests sent: %llu, received: %llu\n",
> +		   gt->info.id, atomic64_read(&gt->tlb_invalidation.sent_count),
> +		   atomic64_read(&gt->tlb_invalidation.received_count));

printing the GT identifier (GT%d) is redundant as path to this debugfs
entry already contains GT identifier:

$ sudo cat  /sys/kernel/debug/dri/0/gt1/tlb_stats
                                    ^^^

GT1, TLB Requests sent: 212, received: 212
^^^

btw, maybe 'last seqno' and 'default timeout' will be worth to be
included in debugfs ?

> +}
> diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h
> index a84065fa324c..f420029ec02d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h
> +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h
> @@ -10,6 +10,7 @@
>  
>  #include "xe_gt_tlb_invalidation_types.h"
>  
> +struct drm_printer;
>  struct xe_gt;
>  struct xe_guc;
>  struct xe_vma;
> @@ -36,4 +37,5 @@ xe_gt_tlb_invalidation_fence_wait(struct xe_gt_tlb_invalidation_fence *fence)
>  	dma_fence_wait(&fence->base, false);
>  }
>  
> +void xe_gt_tlb_dump(struct xe_gt *gt, struct drm_printer *p);

missing empty separation line

>  #endif	/* _XE_GT_TLB_INVALIDATION_ */