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

[John Harrison]

On 7/21/2025 6:35 PM, 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     | 32 +++++++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_guc_log.h     |  1 +
>   3 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_debugfs.c b/drivers/gpu/drm/xe/xe_guc_debugfs.c
> index 0b102ab46c4d..6cbc4240a3bd 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);
> @@ -121,6 +127,7 @@ static const struct drm_info_list slpc_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 },
>   };
>   
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index c01ccb35dc75..d2256773aaf3 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_lfd_abi.h"
>   #include "regs/xe_guc_regs.h"
>   #include "xe_bo.h"
>   #include "xe_devcoredump.h"
> @@ -19,6 +20,12 @@
>   #include "xe_mmio.h"
>   #include "xe_module.h"
>   
> +static const struct guc_lfd_file_header default_guc_lfd_file_header = {
> +	.magic = GUC_LFD_DRIVER_KEY_STREAMING,
> +	.version.dw0 = FIELD_PREP_CONST(GUC_LFD_VERSION_MINOR, GUC_LFD_FORMAT_VERSION_MINOR) |
> +		       FIELD_PREP_CONST(GUC_LFD_VERSION_MAJOR, GUC_LFD_FORMAT_VERSION_MAJOR)
> +};
For the sake of two words, I'm not seeing that it is worth having this 
as a static global structure. You are going to need to either copy it to 
a local structure that you then fill in the 'lfd_stream' field of or 
only output the first part of the structure. Either option is messy.

> +
>   static struct xe_guc *
>   log_to_guc(struct xe_guc_log *log)
>   {
> @@ -216,6 +223,17 @@ 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)
> +{
> +	if (!snapshot || !snapshot->size)
> +		return;
> +
> +	/* Output LFD file header */
> +	xe_print_blob_ascii85(p, "[LOG].data", 0, &default_guc_lfd_file_header, 0,
> +			      offsetof(struct guc_lfd_file_header, lfd_stream));
This is not an LFD format output. And a quick glance I am not seeing it 
being replaced in any of the later patches.

Also, all you are generating at this point is a header with no actual 
LFD blocks. That would not be a valid LFD stream. You should move this 
patch to much later in the series (if not the end) so you only start 
generating a file when it is a valid file.

> +}
> +
>   /**
>    * xe_guc_log_print_dmesg - dump a copy of the GuC log to dmesg
>    * @log: GuC log structure
> @@ -251,6 +269,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
Is this a cut/paste comment from somewhere else?

John.

> + * @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);
> +	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 f1e2b0be90a9..0f6299886010 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);