[PATCH v2 1/3] drm/xe: Remove ci-only GuC FW definitions

[Daniele Ceraolo Spurio]

On 1/30/2024 2:11 PM, Dixit, Ashutosh wrote:
> On Tue, 30 Jan 2024 10:48:50 -0800, Daniele Ceraolo Spurio wrote:
>> On 1/30/2024 10:33 AM, Dixit, Ashutosh wrote:
>>> On Tue, 19 Dec 2023 12:00:20 -0800, Daniele Ceraolo Spurio wrote:
>>>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
>>>> index 73d6938c921d..9dff96dfe455 100644
>>>> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
>>>> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>>>> @@ -102,9 +102,7 @@ struct fw_blobs_by_type {
>>>>    };
>>>>
>>>>    #define XE_GUC_FIRMWARE_DEFS(fw_def, mmp_ver, major_ver)			\
>>>> -	fw_def(LUNARLAKE,	mmp_ver(xe,	guc,	lnl,	70, 6, 8))	\
>>>> 	fw_def(METEORLAKE,	major_ver(i915,	guc,	mtl,	70, 7))		\
>>>> -	fw_def(PVC,		mmp_ver(xe,	guc,	pvc,	70, 9, 1))	\
>>>> 	fw_def(DG2,		major_ver(i915,	guc,	dg2,	70, 5))		\
>>>> 	fw_def(DG1,		major_ver(i915,	guc,	dg1,	70, 5))		\
>>>> 	fw_def(ALDERLAKE_N,	major_ver(i915,	guc,	tgl,	70, 5))		\
>>> I think this commit should include an error message saying FW not found (or
>>> whatever). A lot of people have spent a lot of time because probe just
>>> fails with "xe: probe of 0000:00:02.0 failed with error -22" without any
>>> indication of why?
>> There already is a drm_notice for the firmware fetch failure, in
>> uc_fw_request(). Are you say that's not coming out in dmesg?
> Because from what I am seeing, xe_uc_fw_init() is not failing, xe_guc_init
> is returning from if (!xe_uc_fw_is_enabled()):
>
> int xe_guc_init(struct xe_guc *guc)
> {
>          struct xe_device *xe = guc_to_xe(guc);
>          struct xe_gt *gt = guc_to_gt(guc);
>          int ret;
>
>          guc->fw.type = XE_UC_FW_TYPE_GUC;
>          ret = xe_uc_fw_init(&guc->fw);
>          if (ret)
>                  goto out;
>
>          if (!xe_uc_fw_is_enabled(&guc->fw))
>                  return 0;
>
> And finally xe_uc_init fails, not because xe_guc_init fails, but because
> xe_wopcm_init fails :/

Oh yes sorry, it's not a fetch failure, it is a "fw not defined" 
failure. This is a bit more tricky, because the fact that the FW is not 
defined is used to mark that we don't support it yet, which is currently 
not an error (a lack of HuC or GSC is not fatal, so the common code 
can't throw errors there). Maybe we can add an extra check and print an 
error if force_execlist is not set and we don't find the GuC FW?

Daniele