Re: [PATCH 1/3] drm/xe/uc: Report DMA failure using SIGID

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

On 8/27/2026 2:27 AM, Daniele Ceraolo Spurio wrote:
> Convert the uC DMA failure error to use the xe_log_err() helper.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> Cc: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Cc: Alan Previn Teres Alexis <alan.previn.teres.alexis@intel.com>
> Cc: Julia Filipchuk <julia.filipchuk@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_uc_fw.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index e29878b255ba..02ed4f0eaf40 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -18,6 +18,7 @@
>  #include "xe_gt_sriov_vf.h"
>  #include "xe_gt_types.h"
>  #include "xe_guc.h"
> +#include "xe_log.h"
>  #include "xe_map.h"
>  #include "xe_mmio.h"
>  #include "xe_module.h"
> @@ -914,6 +915,7 @@ static int uc_fw_xfer(struct xe_uc_fw *uc_fw, u32 offset, u32 dma_flags)
>  int xe_uc_fw_upload(struct xe_uc_fw *uc_fw, u32 offset, u32 dma_flags)
>  {
>  	struct xe_device *xe = uc_fw_to_xe(uc_fw);
> +	struct xe_gt *gt = uc_fw_to_gt(uc_fw);
>  	int err;

btw, as this function is for GuC & HuC only, maybe we should add:

	xe_gt_assert(gt, uc_fw->type == XE_UC_FW_TYPE_GUC ||
			 uc_fw->type == XE_UC_FW_TYPE_HUC);

>  
>  	/* make sure the status was cleared the last time we reset the uc */
> @@ -931,9 +933,11 @@ int xe_uc_fw_upload(struct xe_uc_fw *uc_fw, u32 offset, u32 dma_flags)
>  	return 0;
>  
>  fail:
> -	drm_err(&xe->drm, "Failed to load %s firmware %s (%d)\n",
> -		xe_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> -		err);
> +	if (uc_fw->type == XE_UC_FW_TYPE_GUC)
> +		xe_log_err(gt, GUC, err, "Failed to load firmware %s\n", uc_fw->path);
> +	else if (uc_fw->type == XE_UC_FW_TYPE_HUC)
> +		xe_log_err(gt, HUC, err, "Failed to load firmware %s\n", uc_fw->path);

so we could either replace above "else if" with "else"
or add at final "else" some 'not-possible' assert ?

> +
>  	xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_LOAD_FAIL);
>  	return err;
>  }

with asserts added,

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