[PATCH v4 3/6] drm/xe/guc: Add new debugfs entry for lfd format output

[Michal Wajdeczko]

On 23.04.2025 23:58, Zhanjun Dong wrote:
> Add new debugfs entry "guc_log_lfd", prepared for output guc log
> in LFD(Log Format Descriptors) format.
> 
> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_debugfs.c |  7 ++++
>  drivers/gpu/drm/xe/xe_guc_log.c     | 52 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_guc_log.h     |  1 +
>  3 files changed, 60 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_debugfs.c b/drivers/gpu/drm/xe/xe_guc_debugfs.c
> index f33013f8a0f3..a441c5775ca3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_guc_debugfs.c
> @@ -85,6 +85,12 @@ static int guc_log(struct xe_guc *guc, struct drm_printer *p)
>  	return 0;
>  }
>  
> +static int guc_log_lfd(struct xe_guc *guc, struct drm_printer *p)
> +{
> +	xe_guc_log_print_lfd(&guc->log, p);
> +	return 0;
> +}
> +
>  static int guc_log_dmesg(struct xe_guc *guc, struct drm_printer *p)
>  {
>  	xe_guc_log_print_dmesg(&guc->log);
> @@ -116,6 +122,7 @@ static const struct drm_info_list vf_safe_debugfs_list[] = {
>  /* everything else should be added here */
>  static const struct drm_info_list pf_only_debugfs_list[] = {
>  	{ "guc_log", .show = guc_debugfs_show, .data = guc_log },
> +	{ "guc_log_lfd", .show = guc_debugfs_show, .data = guc_log_lfd },
>  	{ "guc_log_dmesg", .show = guc_debugfs_show, .data = guc_log_dmesg },
>  	{ "guc_pc", .show = guc_debugfs_show, .data = guc_pc },
>  };
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 38039c411387..e362e407f6d5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -9,6 +9,7 @@
>  
>  #include <drm/drm_managed.h>
>  
> +#include "abi/guc_log_lfd_abi.h"
>  #include "regs/xe_guc_regs.h"
>  #include "xe_bo.h"
>  #include "xe_devcoredump.h"
> @@ -19,6 +20,14 @@
>  #include "xe_mmio.h"
>  #include "xe_module.h"
>  
> +static const struct guc_logfile_header default_guc_logfile_header = {
> +	.magic = LFD_DRIVER_KEY_STREAMING,
> +	.version.dw0 = FIELD_PREP_CONST(GUC_LOGFILE_FMT_VER_MINOR_VERSION,
> +					GUC_LOG_FILE_FORMAT_VERSION_MINOR) |
> +		       FIELD_PREP_CONST(GUC_LOGFILE_FMT_VER_MAJOR_VERSION,
> +					GUC_LOG_FILE_FORMAT_VERSION_MAJOR)
> +};
> +
>  static struct xe_guc *
>  log_to_guc(struct xe_guc_log *log)
>  {
> @@ -216,6 +225,35 @@ void xe_guc_log_snapshot_print(struct xe_guc_log_snapshot *snapshot, struct drm_
>  	}
>  }
>  
> +static void
> +xe_guc_log_snapshot_print_lfd(struct xe_guc_log_snapshot *snapshot, struct drm_printer *p,
> +			      struct xe_guc_log *log)

'log' - unused ?

> +{
> +	char *lfd_buf;
> +	int index;

size_t or drop it completely

> +
> +	if (!snapshot || !snapshot->size)
> +		return;
> +
> +	/* Alloc lfd buffer in fixed size */

s/lfd/LFD to be consistent with your comment below

> +	lfd_buf = kzalloc(GUC_LOG_CHUNK_SIZE, GFP_KERNEL);
> +	if (!lfd_buf) {
> +		index = -ENOMEM;
> +		goto lfd_out;

just exit here?

> +	}
> +
> +	/* Fill LFD file header */
> +	memcpy(lfd_buf, &default_guc_logfile_header, sizeof(default_guc_logfile_header));

hmm, why exactly we need to copy this ?
can't we just print directly from default_guc_logfile_header ?

and what's the goal of printing just empty lfd buffer?
add some explanation in the commit message

> +	index = offsetof(struct guc_logfile_header, lfd_stream);
> +	xe_print_blob_ascii85(p, "[LOG].data", 0, lfd_buf, 0, (size_t)index);
> +
> +lfd_out:
> +	drm_printf(p, "\n");
> +	if (index < 0)
> +		drm_printf(p, "ERROR: Out of memory\n");

shouldn't this be in some common format, like:

	"[LOG].error = %pe", ERR_PTR(-ENOMEM)

> +	kfree(lfd_buf);
> +}
> +
>  /**
>   * xe_guc_log_print_dmesg - dump a copy of the GuC log to dmesg
>   * @log: GuC log structure
> @@ -251,6 +289,20 @@ void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p)
>  	xe_guc_log_snapshot_free(snapshot);
>  }
>  
> +/**
> + * xe_guc_log_print_lfd - dump a copy of the GuC log to some useful location
> + * @log: GuC log structure
> + * @p: the printer object to output to
> + */
> +void xe_guc_log_print_lfd(struct xe_guc_log *log, struct drm_printer *p)
> +{
> +	struct xe_guc_log_snapshot *snapshot;
> +
> +	snapshot = xe_guc_log_snapshot_capture(log, false);
> +	xe_guc_log_snapshot_print_lfd(snapshot, p, log);
> +	xe_guc_log_snapshot_free(snapshot);
> +}
> +
>  int xe_guc_log_init(struct xe_guc_log *log)
>  {
>  	struct xe_device *xe = log_to_xe(log);
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.h b/drivers/gpu/drm/xe/xe_guc_log.h
> index 5b896f5fafaf..37ff4d11e6cf 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.h
> +++ b/drivers/gpu/drm/xe/xe_guc_log.h
> @@ -40,6 +40,7 @@ struct xe_device;
>  
>  int xe_guc_log_init(struct xe_guc_log *log);
>  void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p);
> +void xe_guc_log_print_lfd(struct xe_guc_log *log, struct drm_printer *p);
>  void xe_guc_log_print_dmesg(struct xe_guc_log *log);
>  struct xe_guc_log_snapshot *xe_guc_log_snapshot_capture(struct xe_guc_log *log, bool atomic);
>  void xe_guc_log_snapshot_print(struct xe_guc_log_snapshot *snapshot, struct drm_printer *p);