Re: [PATCH v4 4/4] drm/xe/gt: Report GT reset and power failures using SIGID

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 8/28/2026 11:58 AM, Arvind Yadav wrote:
> Route the GT reset failure log through the structured SIGID logging
> helper.
> 
> Failure to clear GRDOM_FULL means the GT reset did not complete as
> expected. Use the GT component, which maps to XE_SIGID_GT_TDR, and
> pass the reset errno value to the helper.
> 
> Keep the lower-level GRDOM_FULL failure report because it records
> the exact reset register wait that failed before the higher-level
> reset failure is reported.
> 
> Also route GT suspend, resume and runtime power-flow failures through
> the same helper.

can't we do that PM flows in a separate patch?
> 
> These paths were already reported as GT errors. This change only adds
> structured SIGID reporting and keeps the errno in the helper data.
> 
> Cc: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Suggested-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 6805e0d3bf21..58df31ecb825 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -824,8 +824,7 @@ static int do_gt_reset(struct xe_gt *gt)
>  	xe_mmio_write32(&gt->mmio, GDRST, GRDOM_FULL);
>  	err = xe_mmio_wait32(&gt->mmio, GDRST, GRDOM_FULL, 0, 5000, NULL, false);
>  	if (err)
> -		xe_gt_err(gt, "failed to clear GRDOM_FULL (%pe)\n",
> -			  ERR_PTR(err));
> +		xe_log_err(gt, GT, err, "failed to clear GRDOM_FULL\n");

while around, can we improve the error message here?
we are not clearing that bit, the CP does (Bspec:52549)

	"full graphics reset not completed in %u ms"

>  
>  	xe_gsc_wa_14015076503(gt, false);
>  
> @@ -1013,13 +1012,13 @@ int xe_gt_suspend(struct xe_gt *gt)

hmm, for the PM flows below, is the SIGID GT_TDR correct one?

shouldn't we use PCI component (or maybe new sub-component POWER)
that uses XE_SIGID_IO_BUS ?

>  
>  	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>  	if (!xe_force_wake_ref_has_domain(fw_ref.domains, XE_FORCEWAKE_ALL)) {
> -		xe_gt_err(gt, "suspend failed (%pe)\n", ERR_PTR(-ETIMEDOUT));
> +		xe_log_err(gt, GT, -ETIMEDOUT, "suspend failed\n");

btw, maybe it's a time to improve our error message:

	"can't suspend due to forcewake error"

>  		return -ETIMEDOUT;
>  	}
>  
>  	err = xe_uc_suspend(&gt->uc);
>  	if (err) {
> -		xe_gt_err(gt, "suspend failed (%pe)\n", ERR_PTR(err));
> +		xe_log_err(gt, GT, err, "suspend failed\n");

	"can't suspend due to uc error"

>  		return err;
>  	}
>  
> @@ -1065,7 +1064,7 @@ int xe_gt_resume(struct xe_gt *gt)
>  	xe_gt_dbg(gt, "resuming\n");
>  	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>  	if (!xe_force_wake_ref_has_domain(fw_ref.domains, XE_FORCEWAKE_ALL)) {
> -		xe_gt_err(gt, "resume failed (%pe)\n", ERR_PTR(-ETIMEDOUT));
> +		xe_log_err(gt, GT, -ETIMEDOUT, "resume failed\n");

	"can't resume due to forcewake error"

>  		return -ETIMEDOUT;
>  	}
>  
> @@ -1092,7 +1091,7 @@ int xe_gt_runtime_suspend(struct xe_gt *gt)
>  
>  	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>  	if (!xe_force_wake_ref_has_domain(fw_ref.domains, XE_FORCEWAKE_ALL)) {
> -		xe_gt_err(gt, "runtime suspend failed (%pe)\n", ERR_PTR(-ETIMEDOUT));
> +		xe_log_err(gt, GT, -ETIMEDOUT, "runtime suspend failed\n");

	"can't runtime suspend due to forcewake error"

>  		return -ETIMEDOUT;
>  	}
>  
> @@ -1116,7 +1115,7 @@ int xe_gt_runtime_resume(struct xe_gt *gt)
>  
>  	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>  	if (!xe_force_wake_ref_has_domain(fw_ref.domains, XE_FORCEWAKE_ALL)) {
> -		xe_gt_err(gt, "runtime resume failed (%pe)\n", ERR_PTR(-ETIMEDOUT));
> +		xe_log_err(gt, GT, -ETIMEDOUT, "runtime resume failed\n");

	"can't runtime resume due to forcewake error"

>  		return -ETIMEDOUT;
>  	}
>