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

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Matt Roper
Sent: Thursday, August 15, 2024 10:26 AM
To: intel-xe at lists.freedesktop.org
Cc: Roper, Matthew D <matthew.d.roper at intel.com>
Subject: [PATCH 1/2] drm/xe: Add debugfs to dump GuC's hwconfig
> 
> 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.
> 
> Signed-off-by: Matt Roper <matthew.d.roper at 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.
> +		 */

I think there's an argument to be made that since we can't be certain what
the format of the single dword attribute is, any format we choose would be
just as valid as any other.  So maybe it would be best if we just returned a
hexadecimal representation for every attribute value, rather than having
an exception for the single dword case?

Not blocking, just a consideration.

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

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