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

[Lucas De Marchi]

On Fri, Oct 13, 2023 at 11:28:28AM -0700, Daniele Ceraolo Spurio wrote:
>
>
>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 

that sounds like the normal way to identify file types... check a marker
and adapt the parser according to it.

>cons of this approach is that if the binary gets corrupted (which can 
>rarely happen) we might get an unclear error through the fallback.

I don't think there's much problem with that... if the file is
corrupt, returning an error in the gsc or ccs format handler would be
sufficient, it doesn't matter which.

>Would this work for you?

yes, it seems better.

thanks
Lucas De Marchi

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