Re: [Intel-gfx] [PATCH 1/1] drm/i915/uc: Update to latest GuC and use new-format GuC/HuC names

[John Harrison <john.c.harrison@intel.com>]

On 9/15/2022 01:59, Tvrtko Ursulin wrote:
>
> Hi,
>
> On 15/09/2022 00:46, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Going forwards, the intention is for GuC firmware files to be named
>> for their major version only and HuC firmware files to have no version
>> number in the name at all. This patch adds those entries for all
>> platforms that are officially GuC/HuC enabled.
>>
>> Also, update the expected GuC version numbers to the latest firmware
>> release for those platforms.
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> index 1169e2a09da24..b91ad4aede1f7 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -72,12 +72,14 @@ void intel_uc_fw_change_status(struct intel_uc_fw 
>> *uc_fw,
>>    * security fixes, etc. to be enabled.
>>    */
>>   #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
>> -    fw_def(DG2,          0, guc_mmp(dg2,  70, 4, 1)) \
>> +    fw_def(DG2,          0, guc_maj(dg2,  70, 5)) \
>
> Just glancing over out of curiosity. Part which confused me is that if 
> only major is supposed to be used then what is the '5' in guc_maj(dg2, 
> 70, 5) ?
See the earlier patch that added support for version reduced filenames. 
The minor number is still specified because want to be able to warn the 
user if their firmware is out of date and causing them to miss features, 
security fixes, etc. The driver will still load any old firmware with 
the right name and work with it, but user's need to know that there are 
updates available.

>
> I also couldn't find guc_maj with grep so I guess it's some sort of a 
> magic concatenation macro or what?
'guc_maj' is a macro parameter as per the definition of the macro three 
lines above. According to where INTEL_GUC_FIRMWARE_DEFS is used, it 
becomes either a mechanism for creating just a 'MODULE_FIRMWARE' 
definition for the firmware file or a table entry giving all the version 
information as well as the filename.

John.


>
> Regards,
>
> Tvrtko
>
>> +    fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5)) \
>>       fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 70, 1, 1)) \
>>       fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 69, 0, 3)) \
>> +    fw_def(ALDERLAKE_S,  0, guc_maj(tgl,  70, 5)) \
>>       fw_def(ALDERLAKE_S,  0, guc_mmp(tgl,  70, 1, 1)) \
>>       fw_def(ALDERLAKE_S,  0, guc_mmp(tgl,  69, 0, 3)) \
>> -    fw_def(DG1,          0, guc_mmp(dg1,  70, 1, 1)) \
>> +    fw_def(DG1,          0, guc_maj(dg1,  70, 5)) \
>>       fw_def(ROCKETLAKE,   0, guc_mmp(tgl,  70, 1, 1)) \
>>       fw_def(TIGERLAKE,    0, guc_mmp(tgl,  70, 1, 1)) \
>>       fw_def(JASPERLAKE,   0, guc_mmp(ehl,  70, 1, 1)) \
>> @@ -92,9 +94,11 @@ void intel_uc_fw_change_status(struct intel_uc_fw 
>> *uc_fw,
>>       fw_def(SKYLAKE,      0, guc_mmp(skl,  70, 1, 1))
>>     #define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \
>> +    fw_def(ALDERLAKE_P,  0, huc_raw(tgl)) \
>>       fw_def(ALDERLAKE_P,  0, huc_mmp(tgl,  7, 9, 3)) \
>> +    fw_def(ALDERLAKE_S,  0, huc_raw(tgl)) \
>>       fw_def(ALDERLAKE_S,  0, huc_mmp(tgl,  7, 9, 3)) \
>> -    fw_def(DG1,          0, huc_mmp(dg1,  7, 9, 3)) \
>> +    fw_def(DG1,          0, huc_raw(dg1)) \
>>       fw_def(ROCKETLAKE,   0, huc_mmp(tgl,  7, 9, 3)) \
>>       fw_def(TIGERLAKE,    0, huc_mmp(tgl,  7, 9, 3)) \
>>       fw_def(JASPERLAKE,   0, huc_mmp(ehl,  9, 0, 0)) \