| Message ID | 20250210233254.419587-4-alan.previn.teres.alexis@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Maintenence of devcoredump <-> GuC-Err-Capture plumbing | expand |
On 2025-02-10 6:32 p.m., Alan Previn wrote: > Relocate the xe_engine_snapshot_print function from xe_guc_capture.c > into xe_hw_engine.c but split out the GuC-Err-Capture register printing > portion out into a separate helper inside xe_guc_capture.c so that > we can have a clear separation between printing the general engine info > vs GuC-Err-Capture node's register list. > > v7: - Fix function name to respect "xe_hw_engine" name space. (Rodrigo) > - Remove additional newline in engine dump (Jose Souza) + > ensure changes didn't break mesa's aubinator tool (Rodrigo) > > Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com> > --- > drivers/gpu/drm/xe/xe_devcoredump.c | 2 +- > drivers/gpu/drm/xe/xe_guc_capture.c | 79 +++++++++++++---------------- > drivers/gpu/drm/xe/xe_guc_capture.h | 4 +- > drivers/gpu/drm/xe/xe_hw_engine.c | 29 ++++++++++- > drivers/gpu/drm/xe/xe_hw_engine.h | 1 + > 5 files changed, 67 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c > index 006041997550..7a4610d2ea4f 100644 > --- a/drivers/gpu/drm/xe/xe_devcoredump.c > +++ b/drivers/gpu/drm/xe/xe_devcoredump.c > @@ -128,7 +128,7 @@ static ssize_t __xe_devcoredump_read(char *buffer, size_t count, > drm_puts(&p, "\n**** HW Engines ****\n"); > for (i = 0; i < XE_NUM_HW_ENGINES; i++) > if (ss->hwe[i]) > - xe_engine_snapshot_print(ss->hwe[i], &p); > + xe_hw_engine_snapshot_print(ss->hwe[i], &p); > > drm_puts(&p, "\n**** VM state ****\n"); > xe_vm_snapshot_print(ss->vm, &p); > diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c > index f118e8dd0ecb..76c20ff97864 100644 > --- a/drivers/gpu/drm/xe/xe_guc_capture.c > +++ b/drivers/gpu/drm/xe/xe_guc_capture.c > @@ -917,9 +917,10 @@ guc_capture_init_node(struct xe_guc *guc, struct xe_guc_capture_snapshot *node) > * -------------------- > * --> xe_devcoredump_read-> > * L--> xxx_snapshot_print > - * L--> xe_engine_snapshot_print > - * Print register lists values saved at > - * guc->capture->outlist > + * L--> xe_hw_engine_print --> xe_hw_engine_snapshot_print > + * L--> xe_guc_capture_snapshot_print > + * Print register lists values saved in matching > + * node from guc->capture->outlist > * > */ > > @@ -1655,22 +1656,16 @@ guc_capture_find_reg(struct gcap_reg_list_info *reginfo, u32 addr, u32 flags) > } > > static void > -snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p, > - u32 type, const struct __guc_mmio_reg_descr_group *list) > +print_noderegs_by_list_order(struct xe_guc *guc, struct gcap_reg_list_info *reginfo, > + const struct __guc_mmio_reg_descr_group *list, struct drm_printer *p) > { > - struct xe_gt *gt = snapshot->hwe->gt; > - struct xe_guc *guc = >->uc.guc; > - struct gcap_reg_list_info *reginfo = NULL; > - u32 i, last_value = 0; > + u32 last_value, i; > bool is_ext, low32_ready = false; > > if (!list || !list->list || list->num_regs == 0) > return; > > - XE_WARN_ON(!snapshot->matched_node); > - > is_ext = list == guc->capture->extlists; > - reginfo = &snapshot->matched_node->reginfo[type]; > > /* > * loop through descriptor first and find the register in the node > @@ -1740,8 +1735,8 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ > > group = FIELD_GET(GUC_REGSET_STEERING_GROUP, reg_desc->flags); > instance = FIELD_GET(GUC_REGSET_STEERING_INSTANCE, reg_desc->flags); > - dss = xe_gt_mcr_steering_info_to_dss_id(gt, group, instance); > - > + dss = xe_gt_mcr_steering_info_to_dss_id(guc_to_gt(guc), group, > + instance); > drm_printf(p, "\t%s[%u]: 0x%08x\n", reg_desc->regname, dss, value); > } else { > drm_printf(p, "\t%s: 0x%08x\n", reg_desc->regname, value); > @@ -1760,13 +1755,18 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ > } > > /** > - * xe_engine_snapshot_print - Print out a given Xe HW Engine snapshot. > - * @snapshot: Xe HW Engine snapshot object. > + * xe_guc_capture_snapshot_print - Print out a the contents of a provided Guc-Err-Capture node > + * @guc : Target GuC for operation. > + * @node: GuC Error Capture register dump node. > * @p: drm_printer where it will be printed out. > * > - * This function prints out a given Xe HW Engine snapshot object. > + * This function prints out a register dump of a GuC-Err-Capture node that was retrieved > + * earlier either by GuC-FW reporting or by manual capture depending on how the > + * caller (typically xe_hw_engine_snapshot) was invoked and used. > */ > -void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p) > + > +void xe_guc_capture_snapshot_print(struct xe_guc *guc, struct xe_guc_capture_snapshot *node, > + struct drm_printer *p) > { > const char *grptype[GUC_STATE_CAPTURE_GROUP_TYPE_MAX] = { > "full-capture", > @@ -1774,45 +1774,36 @@ void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm > }; > int type; > const struct __guc_mmio_reg_descr_group *list; > - enum guc_capture_list_class_type capture_class; > struct xe_gt *gt; > > - if (!snapshot) > + if (!guc) > return; > - > - gt = snapshot->hwe->gt; > - > - if (!snapshot->matched_node) > + gt = guc_to_gt(guc); > + if (!node) { node was called snapshot befrore. > + xe_gt_warn(gt, "GuC Capture printing without node!\n"); > return; > + } > + if (!p) { New printer pointer check, good. > + xe_gt_warn(gt, "GuC Capture printing without printer!\n"); > + return; > + } > > - xe_gt_assert(gt, snapshot->hwe); > - > - capture_class = xe_engine_class_to_guc_capture_class(snapshot->hwe->class); > - > - drm_printf(p, "%s (physical), logical instance=%d\n", > - snapshot->name ? snapshot->name : "", > - snapshot->logical_instance); > drm_printf(p, "\tCapture_source: %s\n", > - snapshot->matched_node->source == XE_ENGINE_CAPTURE_SOURCE_GUC ? > + node->source == XE_ENGINE_CAPTURE_SOURCE_GUC ? > "GuC" : "Manual"); > - drm_printf(p, "\tCoverage: %s\n", grptype[snapshot->matched_node->is_partial]); > - drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n", > - snapshot->forcewake.domain, snapshot->forcewake.ref); > - drm_printf(p, "\tReserved: %s\n", > - str_yes_no(snapshot->kernel_reserved)); > + drm_printf(p, "\tCoverage: %s\n", grptype[node->is_partial]); Yes, I see the printout order was changed: vcs0 (physical), logical instance=0 Capture_source: GuC Coverage: full-capture Forcewake: domain 0x8, ref 1 Reserved: no FORCEWAKE_GT: 0x00000000 to: vcs0 (physical), logical instance=0 Forcewake: domain 0x8, ref 1 Reserved: no Capture_source: GuC Coverage: full-capture FORCEWAKE_GT: 0x00000000 The xe_exec_capture igt test can handle this change, as long as it not cause other tools stop working, I'm fine. Reviewed-by: Zhanjun Dong <zhanjun.dong@intel.com> > > for (type = GUC_STATE_CAPTURE_TYPE_GLOBAL; type < GUC_STATE_CAPTURE_TYPE_MAX; type++) { > list = xe_guc_capture_get_reg_desc_list(gt, GUC_CAPTURE_LIST_INDEX_PF, type, > - capture_class, false); > - snapshot_print_by_list_order(snapshot, p, type, list); > + node->eng_class, false); > + print_noderegs_by_list_order(guc, &node->reginfo[type], list, p); > } > > - if (capture_class == GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE) { > + if (node->eng_class == GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE) { > + type = GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS; > list = xe_guc_capture_get_reg_desc_list(gt, GUC_CAPTURE_LIST_INDEX_PF, > - GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS, > - capture_class, true); > - snapshot_print_by_list_order(snapshot, p, GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS, > - list); > + type, node->eng_class, true); > + print_noderegs_by_list_order(guc, &node->reginfo[type], list, p); > } > > drm_puts(p, "\n"); > diff --git a/drivers/gpu/drm/xe/xe_guc_capture.h b/drivers/gpu/drm/xe/xe_guc_capture.h > index 8ac893c92f19..e67589ab4342 100644 > --- a/drivers/gpu/drm/xe/xe_guc_capture.h > +++ b/drivers/gpu/drm/xe/xe_guc_capture.h > @@ -15,7 +15,6 @@ > struct xe_exec_queue; > struct xe_guc; > struct xe_hw_engine; > -struct xe_hw_engine_snapshot; > > static inline enum guc_capture_list_class_type xe_guc_class_to_capture_class(u16 class) > { > @@ -55,7 +54,8 @@ struct xe_guc_capture_snapshot * > xe_guc_capture_get_matching_and_lock(struct xe_exec_queue *q, > enum xe_guc_capture_snapshot_source srctype); > void xe_guc_capture_snapshot_store_manual_job(struct xe_guc *guc, struct xe_exec_queue *q); > -void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p); > +void xe_guc_capture_snapshot_print(struct xe_guc *guc, struct xe_guc_capture_snapshot *node, > + struct drm_printer *p); > void xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q); > void xe_guc_capture_steered_list_init(struct xe_guc *guc); > void xe_guc_capture_put_matched_nodes(struct xe_guc *guc, struct xe_guc_capture_snapshot *n); > diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c > index 02871d319471..c980a5c84a8b 100644 > --- a/drivers/gpu/drm/xe/xe_hw_engine.c > +++ b/drivers/gpu/drm/xe/xe_hw_engine.c > @@ -907,6 +907,33 @@ void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot) > kfree(snapshot); > } > > +/** > + * xe_hw_engine_snapshot_print - Print out a given Xe HW Engine snapshot. > + * @snapshot: Xe HW Engine snapshot object. > + * @p: drm_printer where it will be printed out. > + * > + * This function prints out a given Xe HW Engine snapshot object. > + */ > +void xe_hw_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p) > +{ > + struct xe_gt *gt; > + > + if (!snapshot) > + return; > + > + gt = snapshot->hwe->gt; > + > + drm_printf(p, "%s (physical), logical instance=%d\n", > + snapshot->name ? snapshot->name : "", > + snapshot->logical_instance); > + drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n", > + snapshot->forcewake.domain, snapshot->forcewake.ref); > + drm_printf(p, "\tReserved: %s\n", > + str_yes_no(snapshot->kernel_reserved)); > + > + xe_guc_capture_snapshot_print(>->uc.guc, snapshot->matched_node, p); > +} > + > /** > * xe_hw_engine_print - Xe HW Engine Print. > * @hwe: Hardware Engine. > @@ -919,7 +946,7 @@ void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p) > struct xe_hw_engine_snapshot *snapshot; > > snapshot = xe_hw_engine_snapshot_capture(hwe, NULL); > - xe_engine_snapshot_print(snapshot, p); > + xe_hw_engine_snapshot_print(snapshot, p); > xe_hw_engine_snapshot_free(snapshot); > } > > diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h b/drivers/gpu/drm/xe/xe_hw_engine.h > index 6b5f9fa2a594..069b32aa7423 100644 > --- a/drivers/gpu/drm/xe/xe_hw_engine.h > +++ b/drivers/gpu/drm/xe/xe_hw_engine.h > @@ -58,6 +58,7 @@ u32 xe_hw_engine_mask_per_class(struct xe_gt *gt, > struct xe_hw_engine_snapshot * > xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe, struct xe_exec_queue *q); > void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot); > +void xe_hw_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p); > void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p); > void xe_hw_engine_setup_default_lrc_state(struct xe_hw_engine *hwe); >
On Tue, 2025-02-11 at 17:48 -0500, Dong, Zhanjun wrote: > > > On 2025-02-10 6:32 p.m., Alan Previn wrote: > > Relocate the xe_engine_snapshot_print function from xe_guc_capture.c > > into xe_hw_engine.c but split out the GuC-Err-Capture register printing > > portion out into a separate helper inside xe_guc_capture.c so that > > we can have a clear separation between printing the general engine info > > vs GuC-Err-Capture node's register list. > > > > v7: - Fix function name to respect "xe_hw_engine" name space. (Rodrigo) > > - Remove additional newline in engine dump (Jose Souza) + > > ensure changes didn't break mesa's aubinator tool (Rodrigo) > > > > Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com> > > --- > > drivers/gpu/drm/xe/xe_devcoredump.c | 2 +- > > drivers/gpu/drm/xe/xe_guc_capture.c | 79 +++++++++++++---------------- > > drivers/gpu/drm/xe/xe_guc_capture.h | 4 +- > > drivers/gpu/drm/xe/xe_hw_engine.c | 29 ++++++++++- > > drivers/gpu/drm/xe/xe_hw_engine.h | 1 + > > 5 files changed, 67 insertions(+), 48 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c > > index 006041997550..7a4610d2ea4f 100644 > > --- a/drivers/gpu/drm/xe/xe_devcoredump.c > > +++ b/drivers/gpu/drm/xe/xe_devcoredump.c > > @@ -128,7 +128,7 @@ static ssize_t __xe_devcoredump_read(char *buffer, size_t count, > > drm_puts(&p, "\n**** HW Engines ****\n"); > > for (i = 0; i < XE_NUM_HW_ENGINES; i++) > > if (ss->hwe[i]) > > - xe_engine_snapshot_print(ss->hwe[i], &p); > > + xe_hw_engine_snapshot_print(ss->hwe[i], &p); > > > > drm_puts(&p, "\n**** VM state ****\n"); > > xe_vm_snapshot_print(ss->vm, &p); > > diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c > > index f118e8dd0ecb..76c20ff97864 100644 > > --- a/drivers/gpu/drm/xe/xe_guc_capture.c > > +++ b/drivers/gpu/drm/xe/xe_guc_capture.c > > @@ -917,9 +917,10 @@ guc_capture_init_node(struct xe_guc *guc, struct xe_guc_capture_snapshot *node) > > * -------------------- > > * --> xe_devcoredump_read-> > > * L--> xxx_snapshot_print > > - * L--> xe_engine_snapshot_print > > - * Print register lists values saved at > > - * guc->capture->outlist > > + * L--> xe_hw_engine_print --> xe_hw_engine_snapshot_print > > + * L--> xe_guc_capture_snapshot_print > > + * Print register lists values saved in matching > > + * node from guc->capture->outlist > > * > > */ > > > > @@ -1655,22 +1656,16 @@ guc_capture_find_reg(struct gcap_reg_list_info *reginfo, u32 addr, u32 flags) > > } > > > > static void > > -snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p, > > - u32 type, const struct __guc_mmio_reg_descr_group *list) > > +print_noderegs_by_list_order(struct xe_guc *guc, struct gcap_reg_list_info *reginfo, > > + const struct __guc_mmio_reg_descr_group *list, struct drm_printer *p) > > { > > - struct xe_gt *gt = snapshot->hwe->gt; > > - struct xe_guc *guc = >->uc.guc; > > - struct gcap_reg_list_info *reginfo = NULL; > > - u32 i, last_value = 0; > > + u32 last_value, i; > > bool is_ext, low32_ready = false; > > > > if (!list || !list->list || list->num_regs == 0) > > return; > > > > - XE_WARN_ON(!snapshot->matched_node); > > - > > is_ext = list == guc->capture->extlists; > > - reginfo = &snapshot->matched_node->reginfo[type]; > > > > /* > > * loop through descriptor first and find the register in the node > > @@ -1740,8 +1735,8 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ > > > > group = FIELD_GET(GUC_REGSET_STEERING_GROUP, reg_desc->flags); > > instance = FIELD_GET(GUC_REGSET_STEERING_INSTANCE, reg_desc->flags); > > - dss = xe_gt_mcr_steering_info_to_dss_id(gt, group, instance); > > - > > + dss = xe_gt_mcr_steering_info_to_dss_id(guc_to_gt(guc), group, > > + instance); > > drm_printf(p, "\t%s[%u]: 0x%08x\n", reg_desc->regname, dss, value); > > } else { > > drm_printf(p, "\t%s: 0x%08x\n", reg_desc->regname, value); > > @@ -1760,13 +1755,18 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ > > } > > > > /** > > - * xe_engine_snapshot_print - Print out a given Xe HW Engine snapshot. > > - * @snapshot: Xe HW Engine snapshot object. > > + * xe_guc_capture_snapshot_print - Print out a the contents of a provided Guc-Err-Capture node > > + * @guc : Target GuC for operation. > > + * @node: GuC Error Capture register dump node. > > * @p: drm_printer where it will be printed out. > > * > > - * This function prints out a given Xe HW Engine snapshot object. > > + * This function prints out a register dump of a GuC-Err-Capture node that was retrieved > > + * earlier either by GuC-FW reporting or by manual capture depending on how the > > + * caller (typically xe_hw_engine_snapshot) was invoked and used. > > */ > > -void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p) > > + > > +void xe_guc_capture_snapshot_print(struct xe_guc *guc, struct xe_guc_capture_snapshot *node, > > + struct drm_printer *p) > > { > > const char *grptype[GUC_STATE_CAPTURE_GROUP_TYPE_MAX] = { > > "full-capture", > > @@ -1774,45 +1774,36 @@ void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm > > }; > > int type; > > const struct __guc_mmio_reg_descr_group *list; > > - enum guc_capture_list_class_type capture_class; > > struct xe_gt *gt; > > > > - if (!snapshot) > > + if (!guc) > > return; > > - > > - gt = snapshot->hwe->gt; > > - > > - if (!snapshot->matched_node) > > + gt = guc_to_gt(guc); > > + if (!node) { > node was called snapshot befrore. alan: yes i did - previously snapshot was one-level up of xe_hw_engine that included other things alongside the matching node from guc-err-capture but with this patch i wanted to streamline this this helper since it only needs access to the node and nothing else. > > + xe_gt_warn(gt, "GuC Capture printing without node!\n"); > > return; > > + } > > + if (!p) { > New printer pointer check, good. > > + xe_gt_warn(gt, "GuC Capture printing without printer!\n"); > > + return; > > + } > > > > - xe_gt_assert(gt, snapshot->hwe); > > - > > - capture_class = xe_engine_class_to_guc_capture_class(snapshot->hwe->class); > > - > > - drm_printf(p, "%s (physical), logical instance=%d\n", > > - snapshot->name ? snapshot->name : "", > > - snapshot->logical_instance); > > drm_printf(p, "\tCapture_source: %s\n", > > - snapshot->matched_node->source == XE_ENGINE_CAPTURE_SOURCE_GUC ? > > + node->source == XE_ENGINE_CAPTURE_SOURCE_GUC ? > > "GuC" : "Manual"); > > - drm_printf(p, "\tCoverage: %s\n", grptype[snapshot->matched_node->is_partial]); > > - drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n", > > - snapshot->forcewake.domain, snapshot->forcewake.ref); > > - drm_printf(p, "\tReserved: %s\n", > > - str_yes_no(snapshot->kernel_reserved)); > > + drm_printf(p, "\tCoverage: %s\n", grptype[node->is_partial]); > Yes, I see the printout order was changed: > vcs0 (physical), logical instance=0 > Capture_source: GuC > Coverage: full-capture > Forcewake: domain 0x8, ref 1 > Reserved: no > FORCEWAKE_GT: 0x00000000 > to: > vcs0 (physical), logical instance=0 > Forcewake: domain 0x8, ref 1 > Reserved: no > Capture_source: GuC > Coverage: full-capture > FORCEWAKE_GT: 0x00000000 > The xe_exec_capture igt test can handle this change, as long as it not > cause other tools stop working, I'm fine. > > Reviewed-by: Zhanjun Dong <zhanjun.dong@intel.com> alan: Thanks for the RB and yes - we tested it with mesa tool also. alan:snip
diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c index 006041997550..7a4610d2ea4f 100644 --- a/drivers/gpu/drm/xe/xe_devcoredump.c +++ b/drivers/gpu/drm/xe/xe_devcoredump.c @@ -128,7 +128,7 @@ static ssize_t __xe_devcoredump_read(char *buffer, size_t count, drm_puts(&p, "\n**** HW Engines ****\n"); for (i = 0; i < XE_NUM_HW_ENGINES; i++) if (ss->hwe[i]) - xe_engine_snapshot_print(ss->hwe[i], &p); + xe_hw_engine_snapshot_print(ss->hwe[i], &p); drm_puts(&p, "\n**** VM state ****\n"); xe_vm_snapshot_print(ss->vm, &p); diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c index f118e8dd0ecb..76c20ff97864 100644 --- a/drivers/gpu/drm/xe/xe_guc_capture.c +++ b/drivers/gpu/drm/xe/xe_guc_capture.c @@ -917,9 +917,10 @@ guc_capture_init_node(struct xe_guc *guc, struct xe_guc_capture_snapshot *node) * -------------------- * --> xe_devcoredump_read-> * L--> xxx_snapshot_print - * L--> xe_engine_snapshot_print - * Print register lists values saved at - * guc->capture->outlist + * L--> xe_hw_engine_print --> xe_hw_engine_snapshot_print + * L--> xe_guc_capture_snapshot_print + * Print register lists values saved in matching + * node from guc->capture->outlist * */ @@ -1655,22 +1656,16 @@ guc_capture_find_reg(struct gcap_reg_list_info *reginfo, u32 addr, u32 flags) } static void -snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p, - u32 type, const struct __guc_mmio_reg_descr_group *list) +print_noderegs_by_list_order(struct xe_guc *guc, struct gcap_reg_list_info *reginfo, + const struct __guc_mmio_reg_descr_group *list, struct drm_printer *p) { - struct xe_gt *gt = snapshot->hwe->gt; - struct xe_guc *guc = >->uc.guc; - struct gcap_reg_list_info *reginfo = NULL; - u32 i, last_value = 0; + u32 last_value, i; bool is_ext, low32_ready = false; if (!list || !list->list || list->num_regs == 0) return; - XE_WARN_ON(!snapshot->matched_node); - is_ext = list == guc->capture->extlists; - reginfo = &snapshot->matched_node->reginfo[type]; /* * loop through descriptor first and find the register in the node @@ -1740,8 +1735,8 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ group = FIELD_GET(GUC_REGSET_STEERING_GROUP, reg_desc->flags); instance = FIELD_GET(GUC_REGSET_STEERING_INSTANCE, reg_desc->flags); - dss = xe_gt_mcr_steering_info_to_dss_id(gt, group, instance); - + dss = xe_gt_mcr_steering_info_to_dss_id(guc_to_gt(guc), group, + instance); drm_printf(p, "\t%s[%u]: 0x%08x\n", reg_desc->regname, dss, value); } else { drm_printf(p, "\t%s: 0x%08x\n", reg_desc->regname, value); @@ -1760,13 +1755,18 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_ } /** - * xe_engine_snapshot_print - Print out a given Xe HW Engine snapshot. - * @snapshot: Xe HW Engine snapshot object. + * xe_guc_capture_snapshot_print - Print out a the contents of a provided Guc-Err-Capture node + * @guc : Target GuC for operation. + * @node: GuC Error Capture register dump node. * @p: drm_printer where it will be printed out. * - * This function prints out a given Xe HW Engine snapshot object. + * This function prints out a register dump of a GuC-Err-Capture node that was retrieved + * earlier either by GuC-FW reporting or by manual capture depending on how the + * caller (typically xe_hw_engine_snapshot) was invoked and used. */ -void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p) + +void xe_guc_capture_snapshot_print(struct xe_guc *guc, struct xe_guc_capture_snapshot *node, + struct drm_printer *p) { const char *grptype[GUC_STATE_CAPTURE_GROUP_TYPE_MAX] = { "full-capture", @@ -1774,45 +1774,36 @@ void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm }; int type; const struct __guc_mmio_reg_descr_group *list; - enum guc_capture_list_class_type capture_class; struct xe_gt *gt; - if (!snapshot) + if (!guc) return; - - gt = snapshot->hwe->gt; - - if (!snapshot->matched_node) + gt = guc_to_gt(guc); + if (!node) { + xe_gt_warn(gt, "GuC Capture printing without node!\n"); return; + } + if (!p) { + xe_gt_warn(gt, "GuC Capture printing without printer!\n"); + return; + } - xe_gt_assert(gt, snapshot->hwe); - - capture_class = xe_engine_class_to_guc_capture_class(snapshot->hwe->class); - - drm_printf(p, "%s (physical), logical instance=%d\n", - snapshot->name ? snapshot->name : "", - snapshot->logical_instance); drm_printf(p, "\tCapture_source: %s\n", - snapshot->matched_node->source == XE_ENGINE_CAPTURE_SOURCE_GUC ? + node->source == XE_ENGINE_CAPTURE_SOURCE_GUC ? "GuC" : "Manual"); - drm_printf(p, "\tCoverage: %s\n", grptype[snapshot->matched_node->is_partial]); - drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n", - snapshot->forcewake.domain, snapshot->forcewake.ref); - drm_printf(p, "\tReserved: %s\n", - str_yes_no(snapshot->kernel_reserved)); + drm_printf(p, "\tCoverage: %s\n", grptype[node->is_partial]); for (type = GUC_STATE_CAPTURE_TYPE_GLOBAL; type < GUC_STATE_CAPTURE_TYPE_MAX; type++) { list = xe_guc_capture_get_reg_desc_list(gt, GUC_CAPTURE_LIST_INDEX_PF, type, - capture_class, false); - snapshot_print_by_list_order(snapshot, p, type, list); + node->eng_class, false); + print_noderegs_by_list_order(guc, &node->reginfo[type], list, p); } - if (capture_class == GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE) { + if (node->eng_class == GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE) { + type = GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS; list = xe_guc_capture_get_reg_desc_list(gt, GUC_CAPTURE_LIST_INDEX_PF, - GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS, - capture_class, true); - snapshot_print_by_list_order(snapshot, p, GUC_STATE_CAPTURE_TYPE_ENGINE_CLASS, - list); + type, node->eng_class, true); + print_noderegs_by_list_order(guc, &node->reginfo[type], list, p); } drm_puts(p, "\n"); diff --git a/drivers/gpu/drm/xe/xe_guc_capture.h b/drivers/gpu/drm/xe/xe_guc_capture.h index 8ac893c92f19..e67589ab4342 100644 --- a/drivers/gpu/drm/xe/xe_guc_capture.h +++ b/drivers/gpu/drm/xe/xe_guc_capture.h @@ -15,7 +15,6 @@ struct xe_exec_queue; struct xe_guc; struct xe_hw_engine; -struct xe_hw_engine_snapshot; static inline enum guc_capture_list_class_type xe_guc_class_to_capture_class(u16 class) { @@ -55,7 +54,8 @@ struct xe_guc_capture_snapshot * xe_guc_capture_get_matching_and_lock(struct xe_exec_queue *q, enum xe_guc_capture_snapshot_source srctype); void xe_guc_capture_snapshot_store_manual_job(struct xe_guc *guc, struct xe_exec_queue *q); -void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p); +void xe_guc_capture_snapshot_print(struct xe_guc *guc, struct xe_guc_capture_snapshot *node, + struct drm_printer *p); void xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q); void xe_guc_capture_steered_list_init(struct xe_guc *guc); void xe_guc_capture_put_matched_nodes(struct xe_guc *guc, struct xe_guc_capture_snapshot *n); diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c index 02871d319471..c980a5c84a8b 100644 --- a/drivers/gpu/drm/xe/xe_hw_engine.c +++ b/drivers/gpu/drm/xe/xe_hw_engine.c @@ -907,6 +907,33 @@ void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot) kfree(snapshot); } +/** + * xe_hw_engine_snapshot_print - Print out a given Xe HW Engine snapshot. + * @snapshot: Xe HW Engine snapshot object. + * @p: drm_printer where it will be printed out. + * + * This function prints out a given Xe HW Engine snapshot object. + */ +void xe_hw_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p) +{ + struct xe_gt *gt; + + if (!snapshot) + return; + + gt = snapshot->hwe->gt; + + drm_printf(p, "%s (physical), logical instance=%d\n", + snapshot->name ? snapshot->name : "", + snapshot->logical_instance); + drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n", + snapshot->forcewake.domain, snapshot->forcewake.ref); + drm_printf(p, "\tReserved: %s\n", + str_yes_no(snapshot->kernel_reserved)); + + xe_guc_capture_snapshot_print(>->uc.guc, snapshot->matched_node, p); +} + /** * xe_hw_engine_print - Xe HW Engine Print. * @hwe: Hardware Engine. @@ -919,7 +946,7 @@ void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p) struct xe_hw_engine_snapshot *snapshot; snapshot = xe_hw_engine_snapshot_capture(hwe, NULL); - xe_engine_snapshot_print(snapshot, p); + xe_hw_engine_snapshot_print(snapshot, p); xe_hw_engine_snapshot_free(snapshot); } diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h b/drivers/gpu/drm/xe/xe_hw_engine.h index 6b5f9fa2a594..069b32aa7423 100644 --- a/drivers/gpu/drm/xe/xe_hw_engine.h +++ b/drivers/gpu/drm/xe/xe_hw_engine.h @@ -58,6 +58,7 @@ u32 xe_hw_engine_mask_per_class(struct xe_gt *gt, struct xe_hw_engine_snapshot * xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe, struct xe_exec_queue *q); void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot); +void xe_hw_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm_printer *p); void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p); void xe_hw_engine_setup_default_lrc_state(struct xe_hw_engine *hwe);
Relocate the xe_engine_snapshot_print function from xe_guc_capture.c into xe_hw_engine.c but split out the GuC-Err-Capture register printing portion out into a separate helper inside xe_guc_capture.c so that we can have a clear separation between printing the general engine info vs GuC-Err-Capture node's register list. v7: - Fix function name to respect "xe_hw_engine" name space. (Rodrigo) - Remove additional newline in engine dump (Jose Souza) + ensure changes didn't break mesa's aubinator tool (Rodrigo) Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com> --- drivers/gpu/drm/xe/xe_devcoredump.c | 2 +- drivers/gpu/drm/xe/xe_guc_capture.c | 79 +++++++++++++---------------- drivers/gpu/drm/xe/xe_guc_capture.h | 4 +- drivers/gpu/drm/xe/xe_hw_engine.c | 29 ++++++++++- drivers/gpu/drm/xe/xe_hw_engine.h | 1 + 5 files changed, 67 insertions(+), 48 deletions(-)