Re: [PATCH 1/2] drm/xe: Add debugfs to dump GuC's hwconfig

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Aug 15, 2024 at 02:38:37PM -0700, Matt Roper wrote:
> On Thu, Aug 15, 2024 at 08:08:45PM +0000, Matthew Brost wrote:
> > On Thu, Aug 15, 2024 at 10:26:03AM -0700, Matt Roper wrote:
> > > Although the query uapi is the official way to get at the GuC's hwconfig
> > > table contents, it's still useful to have a quick debugfs interface to
> > > dump the table in a human-readable format while debugging the driver.
> > > 
> > 
> > Patch itself makes sense. Question though, would this be useful to
> > include devcoredump? If so we should code this with the snap semantics
> > for decoredump. See guc_exec_queue_print() for an example of this.
> 
> You mean include the contents of the hwconfig within the devcoredump
> report?  I don't really have a feel for whether anyone would find that

Yes, exactly - include hwconfig within the devcoredump report.

Don't want to hold up the patch as I said it LGTM but just something to
consider and if Jose (or someone on the KMD) indicates this could be
useful it is easy enough to change to use snap semantics and wire into
devcoredump too.

Matt

> useful or not.  My personal use case is just being able to sit down at
> an unfamiliar machine and 'cat' the debugfs to see what the hwconfig
> characteristics of the GPU are before I start doing experiments.
> 
> +Cc Jose for a UMD perspective on whether or not it would also be useful
> to capture the hwconfig contents in the devcoredump for userspace
> debugging.
> 
> 
> Matt
> 
> > 
> > Matt 
> > 
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_gt_debugfs.c   | 11 ++++++
> > >  drivers/gpu/drm/xe/xe_guc_hwconfig.c | 57 ++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/xe/xe_guc_hwconfig.h |  2 +
> > >  3 files changed, 70 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > index 5125d76ccfac..8f95d3a5949b 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > @@ -19,6 +19,7 @@
> > >  #include "xe_gt_sriov_vf_debugfs.h"
> > >  #include "xe_gt_stats.h"
> > >  #include "xe_gt_topology.h"
> > > +#include "xe_guc_hwconfig.h"
> > >  #include "xe_hw_engine.h"
> > >  #include "xe_lrc.h"
> > >  #include "xe_macros.h"
> > > @@ -270,6 +271,15 @@ static int vecs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> > >  	return 0;
> > >  }
> > >  
> > > +static int hwconfig(struct xe_gt *gt, struct drm_printer *p)
> > > +{
> > > +	xe_pm_runtime_get(gt_to_xe(gt));
> > > +	xe_guc_hwconfig_dump(&gt->uc.guc, p);
> > > +	xe_pm_runtime_put(gt_to_xe(gt));
> > > +
> > > +	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},
> > > @@ -288,6 +298,7 @@ static const struct drm_info_list debugfs_list[] = {
> > >  	{"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},
> > > +	{"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig},
> > >  };
> > >  
> > >  void xe_gt_debugfs_register(struct xe_gt *gt)
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> > > index d9b570a154a2..025bad701556 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> > > @@ -6,6 +6,7 @@
> > >  #include "xe_guc_hwconfig.h"
> > >  
> > >  #include <drm/drm_managed.h>
> > > +#include <drm/drm_print.h>
> > >  
> > >  #include "abi/guc_actions_abi.h"
> > >  #include "xe_bo.h"
> > > @@ -103,3 +104,59 @@ void xe_guc_hwconfig_copy(struct xe_guc *guc, void *dst)
> > >  	xe_map_memcpy_from(xe, dst, &guc->hwconfig.bo->vmap, 0,
> > >  			   guc->hwconfig.size);
> > >  }
> > > +
> > > +void xe_guc_hwconfig_dump(struct xe_guc *guc, struct drm_printer *p)
> > > +{
> > > +	size_t size = xe_guc_hwconfig_size(guc);
> > > +	u32 *hwconfig;
> > > +	u64 num_dw;
> > > +	u32 extra_bytes;
> > > +	int i = 0;
> > > +
> > > +	if (size == 0) {
> > > +		drm_printf(p, "No hwconfig available\n");
> > > +		return;
> > > +	}
> > > +
> > > +	num_dw = div_u64_rem(size, sizeof(u32), &extra_bytes);
> > > +
> > > +	hwconfig = kzalloc(size, GFP_KERNEL);
> > > +	if (!hwconfig) {
> > > +		drm_printf(p, "Error: could not allocate hwconfig memory\n");
> > > +		return;
> > > +	}
> > > +
> > > +	xe_guc_hwconfig_copy(guc, hwconfig);
> > > +
> > > +	/* An entry requires at least three dwords for key, length, value */
> > > +	while (i + 3 <= num_dw) {
> > > +		u32 attribute = hwconfig[i++];
> > > +		u32 len_dw = hwconfig[i++];
> > > +
> > > +		if (i + len_dw > num_dw) {
> > > +			drm_printf(p, "Error: Attribute %u is %u dwords, but only %llu remain\n",
> > > +				   attribute, len_dw, num_dw - i);
> > > +			len_dw = num_dw - i;
> > > +		}
> > > +
> > > +		/*
> > > +		 * If it's a single dword (as most hwconfig attributes are),
> > > +		 * then it's probably a number that makes sense to display
> > > +		 * in decimal form.  In the rare cases where it's more than
> > > +		 * one dword, just print it in hex form and let the user
> > > +		 * figure out how to interpret it.
> > > +		 */
> > > +		if (len_dw == 1)
> > > +			drm_printf(p, "[%2u] = %u\n", attribute, hwconfig[i]);
> > > +		else
> > > +			drm_printf(p, "[%2u] = { %*ph }\n", attribute,
> > > +				   (int)(len_dw * sizeof(u32)), &hwconfig[i]);
> > > +		i += len_dw;
> > > +	}
> > > +
> > > +	if (i < num_dw || extra_bytes)
> > > +		drm_printf(p, "Error: %llu extra bytes at end of hwconfig\n",
> > > +			   (num_dw - i) * sizeof(u32) + extra_bytes);
> > > +
> > > +	kfree(hwconfig);
> > > +}
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.h b/drivers/gpu/drm/xe/xe_guc_hwconfig.h
> > > index b5794d641900..7df315900e1c 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_hwconfig.h
> > > +++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.h
> > > @@ -8,10 +8,12 @@
> > >  
> > >  #include <linux/types.h>
> > >  
> > > +struct drm_printer;
> > >  struct xe_guc;
> > >  
> > >  int xe_guc_hwconfig_init(struct xe_guc *guc);
> > >  u32 xe_guc_hwconfig_size(struct xe_guc *guc);
> > >  void xe_guc_hwconfig_copy(struct xe_guc *guc, void *dst);
> > > +void xe_guc_hwconfig_dump(struct xe_guc *guc, struct drm_printer *p);
> > >  
> > >  #endif
> > > -- 
> > > 2.45.2
> > > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation