[PATCH v4 5/6] drm/xe/guc: Add GuC log event buffer output in LFD format

[Michal Wajdeczko]

On 23.04.2025 23:58, Zhanjun Dong wrote:
> Add GuC log event buffer output in LFD format.
> 
> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_log.c | 87 +++++++++++++++++++++++++++++++--
>  1 file changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 3fdf3823e2d0..5aa9b9dbe3f3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -33,6 +33,7 @@ struct guc_log_buffer_entry_list {
>  };
>  
>  struct guc_log_init_config_save {
> +	u32 version;
>  	/*
>  	 * Array of init config KLVs.
>  	 * Range from GUC_LOG_LIC_TYPE_FIRST to GUC_LOG_LIC_TYPE_LAST
> @@ -336,7 +337,6 @@ static int xe_guc_log_add_os_id(void *buf, int buf_size, u32 id)
>  		return -ENOMEM;
>  
>  	strscpy(os_id->build_version, version, info_len);
> -

unrelated change

>  	return xe_guc_log_add_typed_payload(buf, buf_size, GUC_LFD_TYPE_OS_ID,
>  					   info_len + sizeof(*os_id), os_id);
>  }
> @@ -366,6 +366,7 @@ static void xe_guc_log_load_lic(void *guc_log, struct guc_log_init_config_save *
>  	int i = 0;
>  
>  	memset(config, 0, sizeof(struct guc_log_init_config_save));
> +	config->version = p->version;
>  	while (p) {
>  		for (i = 0; i < GUC_LOG_BUFFER_STATE_HEADER_ENTRY_COUNT; i++) {
>  			if (p->marker[0] == entry_markers[i].key[0] &&
> @@ -380,8 +381,7 @@ static void xe_guc_log_load_lic(void *guc_log, struct guc_log_init_config_save *
>  					p++;
>  				} else {
>  					/* Load log init config */
> -					xe_guc_log_loop_log_init((void *)p,
> -								 config);
> +					xe_guc_log_loop_log_init((void *)p, config);

unrelated change

>  
>  					/* Init config is the last */
>  					return;
> @@ -417,6 +417,86 @@ xe_guc_log_output_lfd_init(char *buf, struct xe_guc_log_snapshot *snapshot,
>  	return index;
>  }
>  
> +static void
> +xe_guc_log_print_over_chunks(struct drm_printer *p, struct xe_guc_log_snapshot *snapshot,
> +			     u32 from, u32 to)

guc_log_snapshot_print_chunks(
			struct xe_guc_log_snapshot *snapshot,
			u32 from, u32 to,
			struct drm_printer *p)

> +{
> +	int chunk_from = from % GUC_LOG_CHUNK_SIZE;
> +	int chunk_id = from / GUC_LOG_CHUNK_SIZE;
> +	int to_chunk_id = to / GUC_LOG_CHUNK_SIZE;
> +	int chunk_to = to % GUC_LOG_CHUNK_SIZE;
> +	int pos = from;
> +
> +	do {
> +		size_t size = (to_chunk_id > chunk_id ? GUC_LOG_CHUNK_SIZE : chunk_to) - chunk_from;
> +
> +		xe_print_blob_ascii85(p, NULL, 0, snapshot->copy[chunk_id], chunk_from, size);
> +		pos += size;
> +		chunk_id++;
> +		chunk_from = 0;
> +	} while (pos < to);
> +}
> +
> +static void
> +xe_guc_log_lfd_print_from_chunks(struct drm_printer *p, struct xe_guc_log_snapshot *snapshot,
> +				 struct guc_log_buffer_entry_list *entry)

I'm not sure if having this function helps,
IMO better to keep this code at the caller, where you are already
accessing the data from the 'entry' in "Calculate data length"

> +{
> +	/* Output data from guc log chunks directly */
> +	if (entry->rd_ptr < entry->wr_ptr) {
> +		xe_guc_log_print_over_chunks(p, snapshot,
> +					     entry->offset + entry->rd_ptr,
> +					     entry->offset + entry->wr_ptr);
> +	} else {
> +		/* print from rd to buf end 1st */
> +		xe_guc_log_print_over_chunks(p, snapshot,
> +					     entry->offset + entry->rd_ptr,
> +					     entry->offset + entry->buf_size);
> +
> +		/* print from buf start to wr */
> +		xe_guc_log_print_over_chunks(p, snapshot, entry->offset,
> +					     entry->offset + entry->wr_ptr);
> +	}
> +}
> +
> +static inline int
> +xe_guc_log_add_log_event(struct drm_printer *p, char *buf, struct xe_guc_log_snapshot *snapshot,
> +			 struct guc_log_init_config_save *config)

guc_log_snapshot_print_events(
			struct xe_guc_log_snapshot *snapshot,
			const struct guc_log_init_config_save *config,
			char *buf,
			struct drm_printer *p)

but do we really need this 'buf' ?
those lfd headers are quite small and could be defined on stack

> +{
> +	struct guc_logfile_lfd_data *lfd = (void *)buf;
> +	struct guc_lfd_data_log_events_buf *events_buf;
> +	struct guc_log_buffer_entry_list *entry;
> +	u32 data_len;
> +	int index;
> +
> +	memset(buf, 0, GUC_LOG_CHUNK_SIZE);
> +	entry = &config->entry[GUC_LOG_BUFFER_STATE_HEADER_ENTRY_LOG];
> +
> +	/* Skip empty log */
> +	if (entry->rd_ptr == entry->wr_ptr)
> +		return 0;
> +
> +	index = xe_guc_log_add_lfd_header(buf, GUC_LOG_CHUNK_SIZE);
> +	lfd->dw0 |= FIELD_PREP(GUC_LOGFILE_LFD_DESC_TYPE, GUC_LFD_TYPE_LOG_EVENTS_BUFFER);
> +	events_buf = (void *)&lfd->data;
> +	events_buf->log_events_format_version = config->version;
> +
> +	/* Adjust to log_format_buf */
> +	data_len = offsetof(struct guc_lfd_data_log_events_buf, log_format_buf);
> +	index += data_len;
> +
> +	/* Calculate data length */
> +	data_len += (entry->wr_ptr + entry->buf_size - entry->rd_ptr) % entry->buf_size;
> +	/* make length u32 aligned */
> +	lfd->desc_dw_size = DIV_ROUND_UP(data_len, sizeof(u32));
> +
> +	/* Output GUC_LFD_TYPE_LOG_EVENTS_BUFFER header */
> +	xe_print_blob_ascii85(p, NULL, 0, buf, 0, (size_t)index);
> +
> +	xe_guc_log_lfd_print_from_chunks(p, snapshot, entry);
> +	/* log event buffer content did not use buf, exclude from size count */
> +	return index;
> +}
> +
>  static void
>  xe_guc_log_snapshot_print_lfd(struct xe_guc_log_snapshot *snapshot, struct drm_printer *p,
>  			      struct xe_guc_log *log)
> @@ -441,6 +521,7 @@ xe_guc_log_snapshot_print_lfd(struct xe_guc_log_snapshot *snapshot, struct drm_p
>  		goto lfd_out;
>  
>  	xe_print_blob_ascii85(p, "[LOG].data", 0, lfd_buf, 0, (size_t)index);
> +	index = xe_guc_log_add_log_event(p, lfd_buf, snapshot, &config);
>  
>  lfd_out:
>  	drm_printf(p, "\n");