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

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

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>

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);