On 5/25/2023 4:48 PM, Teres Alexis, Alan Previn wrote:
Considering the only request i have below is touching up of existing comments (as
far as this patch is concerned), and since the rest of the code looks good, here is
my R-b - but i hope you can anwser my newbie question at the bottom:
Reviewed-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote:
Add FW definition and the matching override modparam.
The GSC FW has both a release version, based on platform and a rolling
counter, and a compatibility version, which is the one tracking
interface changes. Since what we care about is the interface, we use
the compatibility version in the buinary names.
alan :s/buinary/binary
Same as with the GuC, a major version bump indicate a
backward-incompatible change, while a minor version bump indicates a
backward-compatible one, so we use only the former in the file name.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 32 ++++++++++++++++++------
1 file changed, 24 insertions(+), 8 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 36ee96c02d74..531cd172151d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -124,6 +124,18 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
fw_def(BROXTON, 0, huc_mmp(bxt, 2, 0, 0)) \
fw_def(SKYLAKE, 0, huc_mmp(skl, 2, 0, 0))
+/*
+ * The GSC FW has both a release version, based on platform and a rolling
+ * counter, and a compatibility version, which is the one tracking
+ * interface changes. Since what we care about is the interface, we use
+ * the compatibility version in the buinary names.
alan:s/buinary/binary
also, since we will (i hope) be adding several comments (alongside the new
version objects under intel_gsc_uc structure) in the patch #3 about what
their differences are and which one we care about and when they get populated,
perhaps we can minimize the information here and redirect to that other
comment... OR ... we can minimize the comments in patch #3 and redirect here
(will be good to have a single location with detailed explaination in the
comments and a redirect-ptr from the other location since a reader would
most likely stumble onto those questions from either of these locations).
I'll redirect, that makes more sense
+ * Same as with the GuC, a major version bump indicate a
+ * backward-incompatible change, while a minor version bump indicates a
+ * backward-compatible one, so we use only the former in the file name.
+ */
+#define INTEL_GSC_FIRMWARE_DEFS(fw_def, gsc_def) \
+ fw_def(METEORLAKE, 0, gsc_def(mtl, 1, 0))
+
/*
alan:snip
@@ -257,14 +281,6 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
int i;
bool found;
- /*
- * GSC FW support is still not fully in place, so we're not defining
- * the FW blob yet because we don't want the driver to attempt to load
- * it until we're ready for it.
- */
- if (uc_fw->type == INTEL_UC_FW_TYPE_GSC)
- return;
-
alan: more of a newbie question from myself: considering this is a new firmware
we are adding, is there some kind of requirement to provide a link to the patch
targetting the linux firmware repo that is a dependency of this series?
or perhaps we should mention in the series that merge will only happen after
that patch gets merged (with a final rev that includes the patch on
the fw-repo side?). Just trying to understand the process.
We usually merge the kernel patch first and do the pull-request to
linux-firmware immediately after. We did have cases where we change
firmware version between the first rev and the one that gets merged, so
we only go to linux-firmware once we're sure it's not going to change
anymore. Case in point, the GSC team has asked me to hold off in sending
the current blob to linux-firmware because they have some pending fixes
they want to include in the first "official" release.
Daniele
/*
* The only difference between the ADL GuC FWs is the HWConfig support.
* ADL-N does not support HWConfig, so we should use the same binary as
