[PATCH v27 6/6] drm/xe/guc: Save manual engine capture into capture list
Teres Alexis, Alan Previn
alan.previn.teres.alexis at intel.com
Fri Oct 4 17:42:33 UTC 2024
On Fri, 2024-10-04 at 10:38 -0700, Teres Alexis, Alan Previn wrote:
> As per offline conversation just now, since our last thorough review in rev18,
> we notice many trivial changes that are not required at all that got accidentally
> included as a result of ongoing offline debugging work that you forgot to remove.
> I'll highlight them below as part of this review. Looking forward for next rev.
>
>
>
> On Thu, 2024-10-03 at 20:24 -0700, Zhanjun Dong wrote:
> > Save manual engine capture into capture list.
> > This removes duplicate register definitions across manual-capture vs
> > guc-err-capture.
> >
> >
> > -void xe_guc_capture_free(struct xe_guc *guc)
> > +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;
> alan: all the changes in this function doesnt change the implementation, usage or behavior
> since Patch #5 - so we can just move these changes directly into patch #5. The name change is
> the one thing i did want to see since "free" is the opposite of "alloc" but we are not
> freeing content, we are returning a node we had "get" previously.
> >
> > - if (n) {
> > - guc_capture_remove_stale_matches_from_list(guc->capture, n);
> > - guc_capture_free_outlist_node(guc->capture, n);
> > + if (devcoredump->snapshot.matched_node) {
> > + guc_capture_remove_stale_matches_from_list(guc->capture,
> > + devcoredump->snapshot.matched_node);
> > + guc_capture_free_outlist_node(guc->capture, devcoredump->snapshot.matched_node);
> > +
> > + devcoredump->snapshot.matched_node = NULL;
> > }
> > - devcoredump->snapshot.matched_node = NULL;
alan: WRT to bringing this hunk into Pach #5, i am okay with you retaining the RB in Patch #5 if not other changes.
(same as the earlier comment in my last reply of this patch)
> > }
> >
> > /*
> alan:snip
More information about the Intel-xe
mailing list