Re: drm/xe/devcoredump: Drop a FIXME in devcoredump

["Vivekanandan, Balasubramani" <balasubramani.vivekanandan@intel.com>]

On 12.05.2026 14:27, Shekhar Chauhan wrote:
> 
> On 5/12/2026 12:47, Vivekanandan, Balasubramani wrote:
> > On 12.05.2026 11:25, Shekhar Chauhan wrote:
> > > The FIXME says that xe_engine_snapshot_print.. is accessing persistent
> > > driver data, unlike what the FIXME says that it does. Drop the FIXME
> > > since the current code is not going to access the hardware while
> > > dumping.
> > Is there any typo in the above commit message? Did you intent to say
> > 
> > The xe_engine_snapshot_print.. is accessing persistent
> > driver data, unlike what the FIXME says that it does.
> > 
> > After making that correction,
> > 
> > Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> Okay, that can be done (although my initial intent was to keep it as "that"
> to signify what the FIXME said). But, making it as "the" instead of "that"
> also makes sense.
> 
> Since it is a minor change, do you think I should send a new revision for it
> or is this something the maintainer(s) can just take care of while merging?

I can make the change the apply the patch.

Regards,
Bala

> 
> -shekhar
> 
> > 
> > Regards,
> > Bala
> > 
> > 
> > > More details about this patch:
> > > https://patchwork.freedesktop.org/patch/703884/?series=161407&rev=1
> > > The starting two feedbacks make sense and the original patch is wrong
> > > in adding those changes, but the last feedback is the one which
> > > highlights the point.
> > > 
> > > Signed-off-by: Shekhar Chauhan <shekhar.chauhan@intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_guc_capture.c | 6 ------
> > >   1 file changed, 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c
> > > index bc49e40165a3..21f7caf9ea08 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_capture.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
> > > @@ -1841,12 +1841,6 @@ void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm
> > >   		   str_yes_no(snapshot->kernel_reserved));
> > >   	for (type = GUC_STATE_CAPTURE_TYPE_GLOBAL; type < GUC_STATE_CAPTURE_TYPE_MAX; type++) {
> > > -		/*
> > > -		 * FIXME: During devcoredump print we should avoid accessing the
> > > -		 * driver pointers for gt or engine. Printing should be done only
> > > -		 * using the snapshot captured. Here we are accessing the gt
> > > -		 * pointer. It should be fixed.
> > > -		 */
> > >   		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);
> 
> -- 
> Shekhar Chauhan
> Linux Graphics Software Engineer
> Intel Corporation
>