[Intel-xe] [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic
Lucas De Marchi
lucas.demarchi at intel.com
Tue Apr 4 18:59:55 UTC 2023
On Mon, Apr 03, 2023 at 06:09:10PM +0000, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi at intel.com>
>> Sent: Wednesday, March 29, 2023 8:46 PM
>> To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>> Cc: intel-xe at lists.freedesktop.org; Harrison, John C
>> <john.c.harrison at intel.com>; Ceraolo Spurio, Daniele
>> <daniele.ceraolospurio at intel.com>; dri-devel at lists.freedesktop.org; Daniel
>> Vetter <daniel.vetter at ffwll.ch>; Dave Airlie <airlied at redhat.com>
>> Subject: Re: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic
>>
>> On Tue, Mar 28, 2023 at 04:31:13PM -0700, Anusha Srivatsa wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: De Marchi, Lucas <lucas.demarchi at intel.com>
>> >> Sent: Thursday, March 23, 2023 10:18 PM
>> >> To: intel-xe at lists.freedesktop.org
>> >> Cc: Srivatsa, Anusha <anusha.srivatsa at intel.com>; Harrison, John C
>> >> <john.c.harrison at intel.com>; Ceraolo Spurio, Daniele
>> >> <daniele.ceraolospurio at intel.com>; dri-devel at lists.freedesktop.org;
>> >> Daniel Vetter <daniel.vetter at ffwll.ch>; Dave Airlie
>> >> <airlied at redhat.com>; De Marchi, Lucas <lucas.demarchi at intel.com>
>> >> Subject: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic
>> >>
>> >> Update the logic to autoselect GuC/HuC for the platforms with the
>> >> following
>> >> improvements:
>> >>
>> >> - Document what is the firmware file that is expected to be
>> >> loaded and what is checked from blob headers
>> >> - When the platform is under force-probe it's desired to enforce
>> >> the full-version requirement so the correct firmware is used
>> >> before widespread adoption and backward-compatibility
>> >>
>> >Extra line ^
>> >
>> >> commitments
>> >> - Directory from which we expect firmware blobs to be available in
>> >> upstream linux-firmware repository depends on the platform: for
>> >> the ones supported by i915 it uses the i915/ directory, but the ones
>> >> expected to be supported by xe, it's on the xe/ directory. This
>> >> means that for platforms in the intersection, the firmware is
>> >> loaded from a different directory, but that is not much important
>> >> in the firmware repo and it avoids firmware duplication.
>> >>
>> >> - Make the table with the firmware definitions clearly state the
>> >> versions being expected. Now with macros to select the version it's
>> >> possible to choose between full-version/major-version for GuC and
>> >> full-version/no-version for HuC. These are similar to the macros used
>> >> in i915, but implemented in a slightly different way to avoid
>> >> duplicating the macros for each firmware/type and functionality,
>> >> besides adding the support for different directories.
>> >>
>> >> - There is no check added regarding force-probe since xe should
>> >> reuse the same firmware files published for i915 for past
>> >> platforms. This can be improved later with additional
>> >> kunit checking against a hardcoded list of platforms that
>> >Extra line here.
>> >
>> >> falls in this category.
>> >> - As mentioned in the TODO, the major version fallback was not
>> >> implemented before as currently each platform only supports one
>> >> major. That can be easily added later.
>> >>
>> >> - GuC version for MTL and PVC were updated to 70.6.4, using the exact
>> >> full version, while the
>> >>
>> >> After this the GuC firmware used by PVC changes to pvc_guc_70.5.2.bin
>> >> since it's using a file not published yet.
>> >>
>> >> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> >> ---
>> >> drivers/gpu/drm/xe/xe_uc_fw.c | 315 +++++++++++++++++-----------
>> >> drivers/gpu/drm/xe/xe_uc_fw.h | 2 +-
>> >> drivers/gpu/drm/xe/xe_uc_fw_types.h | 7 +
>> >> 3 files changed, 204 insertions(+), 120 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c
>> >> b/drivers/gpu/drm/xe/xe_uc_fw.c index 174c42873ebb..653bc3584cc5
>> >> 100644
>> >> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
>> >> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>> >> @@ -17,6 +17,137 @@
>> >> #include "xe_mmio.h"
>> >> #include "xe_uc_fw.h"
>> >>
>> >> +/*
>> >> + * List of required GuC and HuC binaries per-platform. They must be
>> >> +ordered
>> >> + * based on platform, from newer to older.
>> >> + *
>> >> + * Versioning follows the guidelines from
>> >> + * Documentation/driver-api/firmware/firmware-usage-guidelines.rst.
>> >> +There is a
>> >> + * distinction for platforms being officially supported by the driver or not.
>> >> + * Platforms not available publicly or not yet officially supported
>> >> +by the
>> >> + * driver (under force-probe), use the mmp_ver(): the firmware
>> >> +autoselect logic
>> >> + * will select the firmware from disk with filename that matches the
>> >> +full
>> >> + * "mpp version", i.e. major.minor.patch. mmp_ver() should only be
>> >> +used for
>> >> + * this case.
>> >> + *
>> >> + * For platforms officially supported by the driver, the filename
>> >> +always only
>> >> + * ever contains the major version (GuC) or no version at all (HuC).
>> >> + *
>> >> + * After loading the file, the driver parses the versions embedded in the blob.
>> >> + * The major version needs to match a major version supported by the
>> >> +driver (if
>> >> + * any). The minor version is also checked and a notice emitted to
>> >> +the log if
>> >> + * the version found is smaller than the version wanted. This is
>> >> +done only for
>> >> + * informational purposes so users may have a chance to upgrade, but
>> >> +the driver
>> >> + * still loads and use the older firmware.
>> >> + *
>> >> + * Examples:
>> >> + *
>> >> + * 1) Platform officially supported by i915 - using Tigerlake as example.
>> >> + * Driver loads the following firmware blobs from disk:
>> >> + *
>> >> + * - i915/tgl_guc_<major>.bin
>> >> + * - i915/tgl_huc.bin
>> >> + *
>> >> + * <major> number for GuC is checked that it matches the version inside
>> >> + * the blob. <minor> version is checked and if smaller than the expected
>> >> + * an info message is emitted about that.
>> >> + *
>> >> + * 1) XE_<FUTUREINTELPLATFORM>, still under require_force_probe.
>> >> Using
>> >> + * "wipplat" as a short-name. Driver loads the following firmware blobs
>> >> + * from disk:
>> >> + *
>> >> + * - xe/wipplat_guc_<major>.<minor>.<patch>.bin
>> >> + * - xe/wipplat_huc_<major>.<minor>.<patch>.bin
>> >> + *
>> >> + * <major> and <minor> are checked that they match the version inside
>> >> + * the blob. Both of them need to match exactly what the driver is
>> >> + * expecting, otherwise it fails.
>> >> + *
>> >> + * 3) Platform officially supported by xe and out of force-probe. Using
>> >> + * "plat" as a short-name. Except for the different directory, the
>> >> + * behavior is the same as (1). Driver loads the following firmware
>> >> + * blobs from disk:
>> >> + *
>> >> + * - xe/plat_guc_<major>.bin
>> >> + * - xe/plat_huc.bin
>> >> + *
>> >> + * <major> number for GuC is checked that it matches the version inside
>> >> + * the blob. <minor> version is checked and if smaller than the expected
>> >> + * an info message is emitted about that.
>> >> + *
>> >> + * For the platforms already released with a major version, they
>> >> +should never be
>> >> + * removed from the table. Instead new entries with newer versions
>> >> +may be added
>> >> + * before them, so they take precedence.
>> >> + *
>> >> + * TODO: Currently there's no fallback on major version. That's
>> >> +because xe
>> >> + * driver only supports the one major version of each firmware in the table.
>> >> + * This needs to be fixed when the major version of GuC is updated.
>> >> + */
>> >> +
>> >> +struct uc_fw_entry {
>> >> + enum xe_platform platform;
>> >> + struct {
>> >> + const char *path;
>> >> + u16 major;
>> >> + u16 minor;
>> >> + bool full_ver_required;
>> >> + };
>> >> +};
>> >> +
>> >> +struct fw_blobs_by_type {
>> >> + const struct uc_fw_entry *entries;
>> >> + u32 count;
>> >> +};
>> >> +
>> >> +#define XE_GUC_FIRMWARE_DEFS(fw_def, mmp_ver, major_ver)
>> >> \
>> >> + fw_def(METEORLAKE, mmp_ver( i915, guc, mtl, 70, 6, 4))
>> >> \
>> >> + fw_def(PVC, mmp_ver( xe, guc, pvc, 70, 6, 4))
>> >> \
>> >> + fw_def(ALDERLAKE_P, major_ver(i915, guc, adlp, 70, 5))
>> >> \
>> >> + fw_def(ALDERLAKE_S, major_ver(i915, guc, tgl, 70, 5))
>> >> \
>> >> + fw_def(DG2, major_ver(i915, guc, dg2, 70, 5))
>> >> \
>> >> + fw_def(DG1, major_ver(i915, guc, dg1, 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(ALDERLAKE_S, no_ver(i915, huc, tgl))
>> >> \
>> >> + fw_def(DG1, no_ver(i915, huc, dg1))
>> >> \
>> >> + fw_def(TIGERLAKE, no_ver(i915, huc, tgl))
>> >> +
>> >> +#define MAKE_FW_PATH(dir__, uc__, shortname__, version__)
>> >> \
>> >> + __stringify(dir__) "/" __stringify(shortname__) "_"
>> >> +__stringify(uc__)
>> >> version__ ".bin"
>> >> +
>> >> +#define fw_filename_mmp_ver(dir_, uc_, shortname_, a, b, c)
>> >> \
>> >> + MAKE_FW_PATH(dir_, uc_, shortname_, "_" __stringify(a ## . ## b ## .
>> >> ## c))
>> >> +#define fw_filename_major_ver(dir_, uc_, shortname_, a, b)
>> >> \
>> >> + MAKE_FW_PATH(dir_, uc_, shortname_, "_" __stringify(a)) #define
>> >> +fw_filename_no_ver(dir_, uc_, shortname_)
>> >> \
>> >> + MAKE_FW_PATH(dir_, uc_, shortname_, "")
>> >> +
>> >> +#define uc_fw_entry_mmp_ver(dir_, uc_, shortname_, a, b, c)
>> >> \
>> >> + { fw_filename_mmp_ver(dir_, uc_, shortname_, a, b, c),
>> >> \
>> >> + a, b, true }
>> >> +#define uc_fw_entry_major_ver(dir_, uc_, shortname_, a, b)
>> >> \
>> >> + { fw_filename_major_ver(dir_, uc_, shortname_, a, b),
>> >> \
>> >> + a, b }
>> >Why is b required here?
>>
>> because it is setting the minor in the corresponding struct uc_fw_entry.
>> From the tables above, basically for the rows using major_ver(), it will use up to
>> the major version in the arguments to decide what is the
>> *file* to load. The path for the file is constructed with the macro above, so it
>> can be used by both MODULE_FIRMWARE and by setting the patch in the
>> uc_fw_entry. The same major_ver() is used to fill out the rest of the
>> uc_fw_entry, where we need the minor too.
>>
>> See doucumentation above. Copying the relevant part here:
>>
>> <major> number for GuC is checked that it matches the version inside
>> the blob. <minor> version is checked and if smaller than the expected
>> an info message is emitted about that.
>>
>>
>> Lucas De Marchi
>
>Thanks for the explanation. The auto select logic looks good.
>With the extra lines removed from commit message,
>
> Reviewed-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
thanks, applied
Lucas De Marchi
More information about the dri-devel
mailing list