| Message ID | 20210105211345.6990-1-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/uc: Add function to define defaults for GuC/HuC enable | expand |
On 1/5/2021 1:13 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > There is a module parameter for controlling what GuC/HuC features are > enabled. Setting to -1 means 'use the default'. However, the default > is not well defined, out of date and needs to be different across > platforms. I believe this needs a bit more detail about the change in behavior. -1 used to map to HuC loading on all gen11+ platforms, while after this change it will map to disabled on all current platforms and HuC loading on dg1 and all future platforms. > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > CC: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 28 ++++++++++++++++++++++-- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 7 +----- > drivers/gpu/drm/i915/i915_params.h | 1 + > 3 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > index 6a0452815c41..2c08db58cf12 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > @@ -15,6 +15,29 @@ > static const struct intel_uc_ops uc_ops_off; > static const struct intel_uc_ops uc_ops_on; > > +static void uc_expand_default_options(struct intel_uc *uc) > +{ > + struct drm_i915_private *i915 = uc_to_gt(uc)->i915; > + > + if (i915->params.enable_guc != -1) > + return; > + > + /* Don't enable GuC/HuC on pre-Gen12 */ > + if (INTEL_GEN(i915) < 12) { > + i915->params.enable_guc = 0; > + return; > + } > + > + /* Don't enable GuC/HuC on older Gen12 platforms */ > + if (IS_TIGERLAKE(i915) || IS_ROCKETLAKE(i915)) { > + i915->params.enable_guc = 0; > + return; > + } > + > + /* Default: enable HuC authentication only */ > + i915->params.enable_guc = ENABLE_GUC_LOAD_HUC; > +} > + > /* Reset GuC providing us with fresh state for both GuC and HuC. > */ > static int __intel_uc_reset_hw(struct intel_uc *uc) > @@ -79,8 +102,7 @@ static void __confirm_options(struct intel_uc *uc) > "Incompatible option enable_guc=%d - %s\n", > i915->params.enable_guc, "GuC submission is N/A"); > > - if (i915->params.enable_guc & ~(ENABLE_GUC_SUBMISSION | > - ENABLE_GUC_LOAD_HUC)) > + if (i915->params.enable_guc & ~ENABLE_GUC_MASK) > drm_info(&i915->drm, > "Incompatible option enable_guc=%d - %s\n", > i915->params.enable_guc, "undocumented flag"); > @@ -88,6 +110,8 @@ static void __confirm_options(struct intel_uc *uc) > > void intel_uc_init_early(struct intel_uc *uc) > { > + uc_expand_default_options(uc); > + > intel_guc_init_early(&uc->guc); > intel_huc_init_early(&uc->huc); here there is a call to __confirm_options() that has a check for enable_guc == -1, which can be dropped since we can't reach here with that value anymore. with these addressed: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > > 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 602f1a0bc587..67b06fde1225 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -152,16 +152,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) > uc_fw->path = NULL; > } > } > - > - /* We don't want to enable GuC/HuC on pre-Gen11 by default */ > - if (i915->params.enable_guc == -1 && p < INTEL_ICELAKE) > - uc_fw->path = NULL; > } > > static const char *__override_guc_firmware_path(struct drm_i915_private *i915) > { > - if (i915->params.enable_guc & (ENABLE_GUC_SUBMISSION | > - ENABLE_GUC_LOAD_HUC)) > + if (i915->params.enable_guc & ENABLE_GUC_MASK) > return i915->params.guc_firmware_path; > return ""; > } > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h > index 330c03e2b4f7..f031966af5b7 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -32,6 +32,7 @@ struct drm_printer; > > #define ENABLE_GUC_SUBMISSION BIT(0) > #define ENABLE_GUC_LOAD_HUC BIT(1) > +#define ENABLE_GUC_MASK GENMASK(1, 0) > > /* > * Invoke param, a function-like macro, for each i915 param, with arguments:
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c index 6a0452815c41..2c08db58cf12 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c @@ -15,6 +15,29 @@ static const struct intel_uc_ops uc_ops_off; static const struct intel_uc_ops uc_ops_on; +static void uc_expand_default_options(struct intel_uc *uc) +{ + struct drm_i915_private *i915 = uc_to_gt(uc)->i915; + + if (i915->params.enable_guc != -1) + return; + + /* Don't enable GuC/HuC on pre-Gen12 */ + if (INTEL_GEN(i915) < 12) { + i915->params.enable_guc = 0; + return; + } + + /* Don't enable GuC/HuC on older Gen12 platforms */ + if (IS_TIGERLAKE(i915) || IS_ROCKETLAKE(i915)) { + i915->params.enable_guc = 0; + return; + } + + /* Default: enable HuC authentication only */ + i915->params.enable_guc = ENABLE_GUC_LOAD_HUC; +} + /* Reset GuC providing us with fresh state for both GuC and HuC. */ static int __intel_uc_reset_hw(struct intel_uc *uc) @@ -79,8 +102,7 @@ static void __confirm_options(struct intel_uc *uc) "Incompatible option enable_guc=%d - %s\n", i915->params.enable_guc, "GuC submission is N/A"); - if (i915->params.enable_guc & ~(ENABLE_GUC_SUBMISSION | - ENABLE_GUC_LOAD_HUC)) + if (i915->params.enable_guc & ~ENABLE_GUC_MASK) drm_info(&i915->drm, "Incompatible option enable_guc=%d - %s\n", i915->params.enable_guc, "undocumented flag"); @@ -88,6 +110,8 @@ static void __confirm_options(struct intel_uc *uc) void intel_uc_init_early(struct intel_uc *uc) { + uc_expand_default_options(uc); + intel_guc_init_early(&uc->guc); intel_huc_init_early(&uc->huc); 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 602f1a0bc587..67b06fde1225 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -152,16 +152,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) uc_fw->path = NULL; } } - - /* We don't want to enable GuC/HuC on pre-Gen11 by default */ - if (i915->params.enable_guc == -1 && p < INTEL_ICELAKE) - uc_fw->path = NULL; } static const char *__override_guc_firmware_path(struct drm_i915_private *i915) { - if (i915->params.enable_guc & (ENABLE_GUC_SUBMISSION | - ENABLE_GUC_LOAD_HUC)) + if (i915->params.enable_guc & ENABLE_GUC_MASK) return i915->params.guc_firmware_path; return ""; } diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 330c03e2b4f7..f031966af5b7 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -32,6 +32,7 @@ struct drm_printer; #define ENABLE_GUC_SUBMISSION BIT(0) #define ENABLE_GUC_LOAD_HUC BIT(1) +#define ENABLE_GUC_MASK GENMASK(1, 0) /* * Invoke param, a function-like macro, for each i915 param, with arguments: