[Intel-xe] [PATCH 4/4] drm/xe/huc: Define HuC for MTL

[Daniele Ceraolo Spurio]

On 10/13/2023 11:14 AM, Lucas De Marchi wrote:
> On Fri, Sep 15, 2023 at 03:34:58PM -0700, Daniele Ceraolo Spurio wrote:
>> MTL uses a GSC-enabled binary. We expect all GSC-enabled binaries to be
>> defined with a "_gsc", so we can check for that in the name to
>> determine if the binary has the GSC headers or not.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
>> Cc: John Harrison <John.C.Harrison at Intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_uc_fw.c | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c 
>> b/drivers/gpu/drm/xe/xe_uc_fw.c
>> index f956457d0b11..4e43b16a61d5 100644
>> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>> @@ -113,12 +113,13 @@ struct fw_blobs_by_type {
>>     fw_def(ROCKETLAKE,    major_ver(i915,    guc,    tgl,    70, 
>> 5))        \
>>     fw_def(TIGERLAKE,    major_ver(i915,    guc,    tgl,    70, 5))
>>
>> -#define XE_HUC_FIRMWARE_DEFS(fw_def, mmp_ver, no_ver)                \
>> -    fw_def(DG1,        no_ver(i915,    huc,    dg1)) \
>> -    fw_def(ALDERLAKE_P,    no_ver(i915,    huc, tgl))            \
>> -    fw_def(ALDERLAKE_S,    no_ver(i915,    huc, tgl))            \
>> -    fw_def(ROCKETLAKE,    no_ver(i915,    huc, tgl))            \
>> -    fw_def(TIGERLAKE,    no_ver(i915,    huc,    tgl))
>> +#define XE_HUC_FIRMWARE_DEFS(fw_def, mmp_ver, no_ver)        \
>> +    fw_def(METEORLAKE,    no_ver(i915,    huc_gsc, mtl))        \
>> +    fw_def(DG1,        no_ver(i915,    huc,        dg1)) \
>> +    fw_def(ALDERLAKE_P,    no_ver(i915,    huc, tgl))        \
>> +    fw_def(ALDERLAKE_S,    no_ver(i915,    huc, tgl))        \
>> +    fw_def(ROCKETLAKE,    no_ver(i915,    huc, tgl))        \
>> +    fw_def(TIGERLAKE,    no_ver(i915,    huc,        tgl))
>>
>> #define MAKE_FW_PATH(dir__, uc__, shortname__, version__)            \
>>     __stringify(dir__) "/" __stringify(shortname__) "_" 
>> __stringify(uc__) version__ ".bin"
>> @@ -207,6 +208,10 @@ uc_fw_auto_select(struct xe_device *xe, struct 
>> xe_uc_fw *uc_fw)
>>             uc_fw->major_ver_wanted = entries[i].major;
>>             uc_fw->minor_ver_wanted = entries[i].minor;
>>             uc_fw->full_ver_required = entries[i].full_ver_required;
>> +
>> +            if (strstr(uc_fw->path, "gsc"))
>> +                uc_fw->has_gsc_headers = true;
>
> can we rather get this from the headers / magic or whatever from
> *inside* the firmware? Relying on the filename IMO is not a good
> practice.  If we don't have a proper way to identify, then I think it
> would be better a try/fail approach starting with MTL: try with gsc
> headers starting on MTL and fallback on error.

There is no "official" way of knowing what type of headers are on top of 
the FW, but the GSC headers start with a marker, so I can check if 
that's there and if it is not we can assume it is a CSS header. The cons 
of this approach is that if the binary gets corrupted (which can rarely 
happen) we might get an unclear error through the fallback.
Would this work for you?

Daniele

>
> Lucas De Marchi
>
>> +
>>             break;
>>         }
>>     }
>> -- 
>> 2.41.0
>>