On Tue, Jan 28, 2025 at 10:36:47AM -0800, Alan Previn wrote:
> Since '__guc_capture_parsed_output *' is a handle that
> is retrieved, stored and relinquished by an entity
> external to GuC (i.e. xe_devcoredump), lets rename it to
> something formal without the'__' prefix and export it
> via give a header file.
>
> Signed-off-by: Alan Previn <alan.previn.teres.ale...@intel.com>
> ---
> drivers/gpu/drm/xe/xe_devcoredump_types.h | 2 +-
> drivers/gpu/drm/xe/xe_guc_capture.c | 83 ++++++-------------
> drivers/gpu/drm/xe/xe_guc_capture.h | 2 +-
> .../drm/xe/xe_guc_capture_snapshot_types.h | 53 ++++++++++++
> drivers/gpu/drm/xe/xe_hw_engine.c | 2 +-
> drivers/gpu/drm/xe/xe_hw_engine_types.h | 5 --
> 6 files changed, 81 insertions(+), 66 deletions(-)
> create mode 100644 drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h
>
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump_types.h
> b/drivers/gpu/drm/xe/xe_devcoredump_types.h
> index 1a1d16a96b2d..c94ce21043a8 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump_types.h
> +++ b/drivers/gpu/drm/xe/xe_devcoredump_types.h
> @@ -58,7 +58,7 @@ struct xe_devcoredump_snapshot {
> * this single-node tracker works because devcoredump will always only
> * produce one hw-engine capture per devcoredump event
> */
> - struct __guc_capture_parsed_output *matched_node;
> + struct xe_guc_capture_snapshot *matched_node;
> /** @vm: Snapshot of VM state */
> struct xe_vm_snapshot *vm;
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c
> b/drivers/gpu/drm/xe/xe_guc_capture.c
> index f6d523e4c5fe..e04c87739267 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.c
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
> @@ -26,6 +26,7 @@
> #include "xe_guc_ads.h"
> #include "xe_guc_capture.h"
> #include "xe_guc_capture_types.h"
> +#include "xe_guc_capture_snapshot_types.h"
> #include "xe_guc_ct.h"
> #include "xe_guc_exec_queue_types.h"
> #include "xe_guc_log.h"
> @@ -53,40 +54,6 @@ struct __guc_capture_bufstate {
> u32 wr;
> };
>
> -/*
> - * struct __guc_capture_parsed_output - extracted error capture node
> - *
> - * A single unit of extracted error-capture output data grouped together
> - * at an engine-instance level. We keep these nodes in a linked list.
> - * See cachelist and outlist below.
> - */
> -struct __guc_capture_parsed_output {
> - /*
> - * A single set of 3 capture lists: a global-list
> - * an engine-class-list and an engine-instance list.
> - * outlist in __guc_capture_parsed_output will keep
> - * a linked list of these nodes that will eventually
> - * be detached from outlist and attached into to
> - * xe_codedump in response to a context reset
> - */
> - struct list_head link;
> - bool is_partial;
> - u32 eng_class;
> - u32 eng_inst;
> - u32 guc_id;
> - u32 lrca;
> - u32 type;
> - bool locked;
> - enum xe_hw_engine_snapshot_source_id source;
> - struct gcap_reg_list_info {
> - u32 vfid;
> - u32 num_regs;
> - struct guc_mmio_reg *regs;
> - } reginfo[GUC_STATE_CAPTURE_TYPE_MAX];
> -#define GCAP_PARSED_REGLIST_INDEX_GLOBAL BIT(GUC_STATE_CAPTURE_TYPE_GLOBAL)
> -#define GCAP_PARSED_REGLIST_INDEX_ENGCLASS
> BIT(GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS)
> -};
> -
> /*
> * Define all device tables of GuC error capture register lists
> * NOTE:
> @@ -287,7 +254,7 @@ struct xe_guc_state_capture {
>
> static void
> guc_capture_remove_stale_matches_from_list(struct xe_guc_state_capture *gc,
> - struct __guc_capture_parsed_output
> *node);
> + struct xe_guc_capture_snapshot
> *node);
>
> static const struct __guc_mmio_reg_descr_group *
> guc_capture_get_device_reglist(struct xe_device *xe)
> @@ -841,7 +808,7 @@ static void check_guc_capture_size(struct xe_guc *guc)
> }
>
> static void
> -guc_capture_add_node_to_list(struct __guc_capture_parsed_output *node,
> +guc_capture_add_node_to_list(struct xe_guc_capture_snapshot *node,
> struct list_head *list)
> {
> list_add(&node->link, list);
> @@ -849,7 +816,7 @@ guc_capture_add_node_to_list(struct
> __guc_capture_parsed_output *node,
>
> static void
> guc_capture_add_node_to_outlist(struct xe_guc_state_capture *gc,
> - struct __guc_capture_parsed_output *node)
> + struct xe_guc_capture_snapshot *node)
> {
> guc_capture_remove_stale_matches_from_list(gc, node);
> guc_capture_add_node_to_list(node, &gc->outlist);
> @@ -857,14 +824,14 @@ guc_capture_add_node_to_outlist(struct
> xe_guc_state_capture *gc,
>
> static void
> guc_capture_add_node_to_cachelist(struct xe_guc_state_capture *gc,
> - struct __guc_capture_parsed_output *node)
> + struct xe_guc_capture_snapshot *node)
> {
> guc_capture_add_node_to_list(node, &gc->cachelist);
> }
>
> static void
> guc_capture_free_outlist_node(struct xe_guc_state_capture *gc,
> - struct __guc_capture_parsed_output *n)
> + struct xe_guc_capture_snapshot *n)
> {
> if (n) {
> n->locked = 0;
> @@ -876,9 +843,9 @@ guc_capture_free_outlist_node(struct xe_guc_state_capture
> *gc,
>
> static void
> guc_capture_remove_stale_matches_from_list(struct xe_guc_state_capture *gc,
> - struct __guc_capture_parsed_output
> *node)
> + struct xe_guc_capture_snapshot *node)
> {
> - struct __guc_capture_parsed_output *n, *ntmp;
> + struct xe_guc_capture_snapshot *n, *ntmp;
> int guc_id = node->guc_id;
>
> list_for_each_entry_safe(n, ntmp, &gc->outlist, link) {
> @@ -888,7 +855,7 @@ guc_capture_remove_stale_matches_from_list(struct
> xe_guc_state_capture *gc,
> }
>
> static void
> -guc_capture_init_node(struct xe_guc *guc, struct __guc_capture_parsed_output
> *node)
> +guc_capture_init_node(struct xe_guc *guc, struct xe_guc_capture_snapshot
> *node)
> {
> struct guc_mmio_reg *tmp[GUC_STATE_CAPTURE_TYPE_MAX];
> int i;
> @@ -1067,13 +1034,13 @@ guc_capture_log_get_register(struct xe_guc *guc,
> struct __guc_capture_bufstate *
> return 0;
> }
>
> -static struct __guc_capture_parsed_output *
> +static struct xe_guc_capture_snapshot *
> guc_capture_get_prealloc_node(struct xe_guc *guc)
> {
> - struct __guc_capture_parsed_output *found = NULL;
> + struct xe_guc_capture_snapshot *found = NULL;
>
> if (!list_empty(&guc->capture->cachelist)) {
> - struct __guc_capture_parsed_output *n, *ntmp;
> + struct xe_guc_capture_snapshot *n, *ntmp;
>
> /* get first avail node from the cache list */
> list_for_each_entry_safe(n, ntmp, &guc->capture->cachelist,
> link) {
> @@ -1081,7 +1048,7 @@ guc_capture_get_prealloc_node(struct xe_guc *guc)
> break;
> }
> } else {
> - struct __guc_capture_parsed_output *n, *ntmp;
> + struct xe_guc_capture_snapshot *n, *ntmp;
>
> /*
> * traverse reversed and steal back the oldest node already
> @@ -1100,11 +1067,11 @@ guc_capture_get_prealloc_node(struct xe_guc *guc)
> return found;
> }
>
> -static struct __guc_capture_parsed_output *
> -guc_capture_clone_node(struct xe_guc *guc, struct
> __guc_capture_parsed_output *original,
> +static struct xe_guc_capture_snapshot *
> +guc_capture_clone_node(struct xe_guc *guc, struct xe_guc_capture_snapshot
> *original,
> u32 keep_reglist_mask)
> {
> - struct __guc_capture_parsed_output *new;
> + struct xe_guc_capture_snapshot *new;
> int i;
>
> new = guc_capture_get_prealloc_node(guc);
> @@ -1146,7 +1113,7 @@ guc_capture_extract_reglists(struct xe_guc *guc, struct
> __guc_capture_bufstate *
> struct xe_gt *gt = guc_to_gt(guc);
> struct guc_state_capture_group_header_t ghdr = {0};
> struct guc_state_capture_header_t hdr = {0};
> - struct __guc_capture_parsed_output *node = NULL;
> + struct xe_guc_capture_snapshot *node = NULL;
> struct guc_mmio_reg *regs = NULL;
> int i, numlists, numregs, ret = 0;
> enum guc_state_capture_type datatype;
> @@ -1439,11 +1406,11 @@ void xe_guc_capture_process(struct xe_guc *guc)
> __guc_capture_process_output(guc);
> }
>
> -static struct __guc_capture_parsed_output *
> +static struct xe_guc_capture_snapshot *
> guc_capture_alloc_one_node(struct xe_guc *guc)
> {
> struct drm_device *drm = guc_to_drm(guc);
> - struct __guc_capture_parsed_output *new;
> + struct xe_guc_capture_snapshot *new;
> int i;
>
> new = drmm_kzalloc(drm, sizeof(*new), GFP_KERNEL);
> @@ -1468,7 +1435,7 @@ guc_capture_alloc_one_node(struct xe_guc *guc)
> static void
> __guc_capture_create_prealloc_nodes(struct xe_guc *guc)
> {
> - struct __guc_capture_parsed_output *node = NULL;
> + struct xe_guc_capture_snapshot *node = NULL;
> int i;
>
> for (i = 0; i < PREALLOC_NODES_MAX_COUNT; ++i) {
> @@ -1583,7 +1550,7 @@ xe_engine_manual_capture(struct xe_hw_engine *hwe,
> struct xe_hw_engine_snapshot
> struct xe_devcoredump *devcoredump = &xe->devcoredump;
> enum guc_capture_list_class_type capture_class;
> const struct __guc_mmio_reg_descr_group *list;
> - struct __guc_capture_parsed_output *new;
> + struct xe_guc_capture_snapshot *new;
> enum guc_state_capture_type type;
> u16 guc_id = 0;
> u32 lrca = 0;
> @@ -1849,7 +1816,7 @@ void xe_engine_snapshot_print(struct
> xe_hw_engine_snapshot *snapshot, struct drm
> *
> * Returns: found guc-capture node ptr else NULL
> */
> -struct __guc_capture_parsed_output *
> +struct xe_guc_capture_snapshot *
> xe_guc_capture_get_matching_and_lock(struct xe_exec_queue *q)
> {
> struct xe_hw_engine *hwe;
> @@ -1878,7 +1845,7 @@ xe_guc_capture_get_matching_and_lock(struct
> xe_exec_queue *q)
> }
>
> if (guc_class <= GUC_LAST_ENGINE_CLASS) {
> - struct __guc_capture_parsed_output *n, *ntmp;
> + struct xe_guc_capture_snapshot *n, *ntmp;
> struct xe_guc *guc = &q->gt->uc.guc;
> u16 guc_id = q->guc->id;
> u32 lrca = xe_lrc_ggtt_addr(q->lrc[0]);
> @@ -1931,7 +1898,7 @@ xe_engine_snapshot_capture_for_queue(struct
> xe_exec_queue *q)
> coredump->snapshot.hwe[id] =
> xe_hw_engine_snapshot_capture(hwe, q);
> } else {
> - struct __guc_capture_parsed_output *new;
> + struct xe_guc_capture_snapshot *new;
>
> new = xe_guc_capture_get_matching_and_lock(q);
> if (new) {
> @@ -1965,7 +1932,7 @@ void xe_guc_capture_put_matched_nodes(struct xe_guc
> *guc)
> {
> struct xe_device *xe = guc_to_xe(guc);
> struct xe_devcoredump *devcoredump = &xe->devcoredump;
> - struct __guc_capture_parsed_output *n =
> devcoredump->snapshot.matched_node;
> + struct xe_guc_capture_snapshot *n = devcoredump->snapshot.matched_node;
>
> if (n) {
> guc_capture_remove_stale_matches_from_list(guc->capture, n);
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.h
> b/drivers/gpu/drm/xe/xe_guc_capture.h
> index 20a078dc4b85..046989fba3b1 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.h
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.h
> @@ -50,7 +50,7 @@ size_t xe_guc_capture_ads_input_worst_size(struct xe_guc
> *guc);
> const struct __guc_mmio_reg_descr_group *
> xe_guc_capture_get_reg_desc_list(struct xe_gt *gt, u32 owner, u32 type,
> enum guc_capture_list_class_type
> capture_class, bool is_ext);
> -struct __guc_capture_parsed_output
> *xe_guc_capture_get_matching_and_lock(struct xe_exec_queue *q);
> +struct xe_guc_capture_snapshot *xe_guc_capture_get_matching_and_lock(struct
> xe_exec_queue *q);
> void xe_engine_manual_capture(struct xe_hw_engine *hwe, struct
> xe_hw_engine_snapshot *snapshot);
> void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct
> drm_printer *p);
> void xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q);
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h
> b/drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h
> new file mode 100644
> index 000000000000..0be2c809a7ce
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2021-2024 Intel Corporation
2025
then
Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> + */
> +
> +#ifndef _XE_GUC_CAPTURE_SNAPSHOT_TYPES_H
> +#define _XE_GUC_CAPTURE_SNAPSHOT_TYPES_H
> +
> +#include <linux/types.h>
> +#include <abi/guc_capture_abi.h>
> +
> +struct guc_mmio_reg;
> +
> +enum xe_guc_capture_snapshot_source {
> + XE_ENGINE_CAPTURE_SOURCE_MANUAL,
> + XE_ENGINE_CAPTURE_SOURCE_GUC
> +};
> +
> +/*
> + * struct xe_guc_capture_snapshot - extracted error capture node
> + *
> + * A single unit of extracted error-capture output data grouped together
> + * at an engine-instance level. We keep these nodes in a linked list.
> + * See cachelist and outlist below.
> + */
> +struct xe_guc_capture_snapshot {
> + /*
> + * A single set of 3 capture lists: a global-list
> + * an engine-class-list and an engine-instance list.
> + * outlist in xe_guc_state_capture will keep
> + * a linked list of these nodes that will eventually
> + * be detached from outlist and attached into to
> + * xe_codedump in response to a context reset
> + */
> + struct list_head link;
> + bool is_partial;
> + u32 eng_class;
> + u32 eng_inst;
> + u32 guc_id;
> + u32 lrca;
> + u32 type;
> + bool locked;
> + enum xe_guc_capture_snapshot_source source;
> + struct gcap_reg_list_info {
> + u32 vfid;
> + u32 num_regs;
> + struct guc_mmio_reg *regs;
> + } reginfo[GUC_STATE_CAPTURE_TYPE_MAX];
> +#define GCAP_PARSED_REGLIST_INDEX_GLOBAL BIT(GUC_STATE_CAPTURE_TYPE_GLOBAL)
> +#define GCAP_PARSED_REGLIST_INDEX_ENGCLASS
> BIT(GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS)
> +};
> +
> +#endif
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index fc447751fe78..a99e3160724b 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -843,7 +843,7 @@ struct xe_hw_engine_snapshot *
> xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe, struct xe_exec_queue
> *q)
> {
> struct xe_hw_engine_snapshot *snapshot;
> - struct __guc_capture_parsed_output *node;
> + struct xe_guc_capture_snapshot *node;
>
> if (!xe_hw_engine_is_valid(hwe))
> return NULL;
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_types.h
> b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> index e4191a7a2c31..de69e2628f2f 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_types.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> @@ -152,11 +152,6 @@ struct xe_hw_engine {
> struct xe_hw_engine_group *hw_engine_group;
> };
>
> -enum xe_hw_engine_snapshot_source_id {
> - XE_ENGINE_CAPTURE_SOURCE_MANUAL,
> - XE_ENGINE_CAPTURE_SOURCE_GUC
> -};
> -
> /**
> * struct xe_hw_engine_snapshot - Hardware engine snapshot
> *
> --
> 2.34.1
>
