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

["Yadav, Arvind" <arvind.yadav@intel.com>]

On 28-08-2026 19:48, Michal Wajdeczko wrote:
>
> 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?


Agree. I will keep the PM-flow changes out of this 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 ?


Agreed, I will drop the PM-flow SIGID conversions for now.

>
>>   
>>   	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"


Noted.

>
>>   		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"
Noted.
>
>>   		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"
Noted.
>
>>   		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"
Noted.
>
>>   		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"
Noted, This patch will only convert the GT reset failure path. I will 
drop the PM-flow SIGID conversions for now.

Thanks,
Arvind
>
>>   		return -ETIMEDOUT;
>>   	}
>>