Re: [PATCH v2 1/5] drm/xe/guc: Use different error codes for GuC load errors

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

On 9/1/2026 11:12 PM, Umesh Nerlige Ramappa wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Instead of always returning -EPROTO no matter what goes wrong, use
> different error codes based on the error type.
> 
> v2: split to its own patch
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index c7f8bbd4cb92..5285d4cecbc8 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -1144,8 +1144,8 @@ static void print_load_status_err(struct xe_gt *gt, u32 status)
>   * Check GUC_STATUS looking for known terminal states (either completion or
>   * failure) of either the microkernel status field or the boot ROM status field.
>   *
> - * Returns 1 for successful completion, -1 for failure and 0 for any
> - * intermediate state.
> + * Returns 0 for successful completion, -EBUSY for any intermediate state and
> + * other errno values for failure cases.
>   */
>  static int guc_load_done(struct xe_gt *gt, u32 *status, u32 *tries)
>  {
> @@ -1157,20 +1157,22 @@ static int guc_load_done(struct xe_gt *gt, u32 *status, u32 *tries)
>  
>  	switch (ukernel) {
>  	case XE_GUC_LOAD_STATUS_READY:
> -		return 1;
> +		return 0;
>  	case XE_GUC_LOAD_STATUS_ERROR_DEVID_BUILD_MISMATCH:
>  	case XE_GUC_LOAD_STATUS_GUC_PREPROD_BUILD_MISMATCH:
>  	case XE_GUC_LOAD_STATUS_ERROR_DEVID_INVALID_GUCTYPE:
>  	case XE_GUC_LOAD_STATUS_HWCONFIG_ERROR:
>  	case XE_GUC_LOAD_STATUS_BOOTROM_VERSION_MISMATCH:
> +		return -ENOEXEC;
>  	case XE_GUC_LOAD_STATUS_DPC_ERROR:
>  	case XE_GUC_LOAD_STATUS_EXCEPTION:
> +		return -EIO;
>  	case XE_GUC_LOAD_STATUS_INIT_DATA_INVALID:
>  	case XE_GUC_LOAD_STATUS_MPU_DATA_INVALID:
>  	case XE_GUC_LOAD_STATUS_INIT_MMIO_SAVE_RESTORE_INVALID:
>  	case XE_GUC_LOAD_STATUS_KLV_WORKAROUND_INIT_ERROR:
>  	case XE_GUC_LOAD_STATUS_INVALID_FTR_FLAG:
> -		return -1;
> +		return -EINVAL;
>  	}
>  
>  	switch (bootrom) {
> @@ -1184,7 +1186,7 @@ static int guc_load_done(struct xe_gt *gt, u32 *status, u32 *tries)
>  	case XE_BOOTROM_STATUS_MPUMAP_INCORRECT:
>  	case XE_BOOTROM_STATUS_EXCEPTION:
>  	case XE_BOOTROM_STATUS_PROD_KEY_CHECK_FAILURE:
> -		return -1;
> +		return -ENOEXEC;
>  	}
>  
>  	if (++*tries >= 100) {
> @@ -1197,7 +1199,7 @@ static int guc_load_done(struct xe_gt *gt, u32 *status, u32 *tries)
>  			  *status, ukernel, bootrom);
>  	}
>  
> -	return 0;
> +	return -EBUSY;
>  }
>  
>  static int guc_wait_ucode(struct xe_guc *guc)
> @@ -1213,21 +1215,21 @@ static int guc_wait_ucode(struct xe_guc *guc)
>  	before_freq = xe_guc_pc_get_act_freq(guc_pc);
>  	before = ktime_get();
>  
> -	ret = poll_timeout_us(load_result = guc_load_done(gt, &status, &tries), load_result,
> -			      10 * USEC_PER_MSEC,
> +	ret = poll_timeout_us(load_result = guc_load_done(gt, &status, &tries),
> +			      load_result != -EBUSY, 10 * USEC_PER_MSEC,
>  			      GUC_LOAD_TIMEOUT_SEC * USEC_PER_SEC, false);
>  
>  	delta_ms = ktime_to_ms(ktime_sub(ktime_get(), before));
>  	act_freq = xe_guc_pc_get_act_freq(guc_pc);
>  	cur_freq = xe_guc_pc_get_cur_freq_fw(guc_pc);
>  
> -	if (ret || load_result <= 0) {
> +	if (ret || load_result) {
>  		xe_gt_err(gt, "load failed: status = 0x%08X, time = %lldms, freq = %dMHz (req %dMHz)\n",
>  			  status, delta_ms, xe_guc_pc_get_act_freq(guc_pc),
>  			  xe_guc_pc_get_cur_freq_fw(guc_pc));
>  		print_load_status_err(gt, status);
>  
> -		return -EPROTO;
> +		return ret ?: load_result;
>  	}
>  
>  	if (delta_ms > GUC_LOAD_TIME_WARN_MSEC) {

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>