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 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.

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
>