| Message ID | 20171201103317.48416-5-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Michal Wajdeczko (2017-12-01 10:33:15) > We currently have two module parameters that control GuC: > "enable_guc_loading" and "enable_guc_submission". Whenever > we need submission=1, we also need loading=1. We also need > loading=1 when we want to want to load and verify the HuC. > > Lets combine above module parameters into one "enable_guc" > modparam. New supported bit values are: > > 0=disable GuC (no GuC submission, no HuC) > 1=enable GuC submission > 2=enable HuC load > > Special value "-1" can be used to let driver decide what > option should be enabled for given platform based on > hardware/firmware availability or preference. > > Explicit enabling any of the GuC features makes GuC load > a required step, fallback to non-GuC mode will not be > supported. > > v2: Don't use -EIO > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 5 +- > drivers/gpu/drm/i915/i915_params.c | 11 ++-- > drivers/gpu/drm/i915/i915_params.h | 3 +- > drivers/gpu/drm/i915/intel_uc.c | 100 +++++++++++++++++++++---------------- > 4 files changed, 65 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2c386c7..9162a60 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3238,8 +3238,9 @@ intel_info(const struct drm_i915_private *dev_priv) > #define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv)) > > /* Having a GuC is not the same as using a GuC */ > -#define USES_GUC(dev_priv) (i915_modparams.enable_guc_loading) > -#define USES_GUC_SUBMISSION(dev_priv) (i915_modparams.enable_guc_submission) > +#define USES_GUC(dev_priv) (!!(i915_modparams.enable_guc > 0)) > +#define USES_GUC_SUBMISSION(dev_priv) (!!(i915_modparams.enable_guc & 1)) > +#define USES_HUC(dev_priv) (!!(i915_modparams.enable_guc & 2)) * channelling Joonas Please use a magic name for each bit and so #define USE_GUC_SUBMISSION_BIT 0 #define USES_GUC_SUBMISSION (!!(i915_modparams.enable_guc & BIT(USE_GUC_SUBMISSION_BIT))) Gah, that's so ugly. or static inline bool intel_use_guc_submission(void) { return i915_modparams.enable_guc & BIT(USE_GUC_SUBMISSION_BIT); } which at least stops being so shouty. -Chris
On Fri, 01 Dec 2017 13:36:06 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Michal Wajdeczko (2017-12-01 10:33:15) >> We currently have two module parameters that control GuC: >> "enable_guc_loading" and "enable_guc_submission". Whenever >> we need submission=1, we also need loading=1. We also need >> loading=1 when we want to want to load and verify the HuC. >> >> Lets combine above module parameters into one "enable_guc" >> modparam. New supported bit values are: >> >> 0=disable GuC (no GuC submission, no HuC) >> 1=enable GuC submission >> 2=enable HuC load >> >> Special value "-1" can be used to let driver decide what >> option should be enabled for given platform based on >> hardware/firmware availability or preference. >> >> Explicit enabling any of the GuC features makes GuC load >> a required step, fallback to non-GuC mode will not be >> supported. >> >> v2: Don't use -EIO >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 5 +- >> drivers/gpu/drm/i915/i915_params.c | 11 ++-- >> drivers/gpu/drm/i915/i915_params.h | 3 +- >> drivers/gpu/drm/i915/intel_uc.c | 100 >> +++++++++++++++++++++---------------- >> 4 files changed, 65 insertions(+), 54 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 2c386c7..9162a60 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -3238,8 +3238,9 @@ intel_info(const struct drm_i915_private >> *dev_priv) >> #define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv)) >> >> /* Having a GuC is not the same as using a GuC */ >> -#define USES_GUC(dev_priv) >> (i915_modparams.enable_guc_loading) >> -#define USES_GUC_SUBMISSION(dev_priv) >> (i915_modparams.enable_guc_submission) >> +#define USES_GUC(dev_priv) (!!(i915_modparams.enable_guc > >> 0)) >> +#define USES_GUC_SUBMISSION(dev_priv) (!!(i915_modparams.enable_guc & >> 1)) >> +#define USES_HUC(dev_priv) (!!(i915_modparams.enable_guc & >> 2)) > > * channelling Joonas > > Please use a magic name for each bit and so > > #define USE_GUC_SUBMISSION_BIT 0 I was considering that, but then I decided to follow existing code (see USES_PPGTT* and enable_ppgtt where we use plain numbers only) But if we want to start using magic values, then these values should be defined in i915_params.h and rather in this way: #define ENABLE_GUC_SUBMISSION BIT(0) #define ENABLE_GUC_HUC_LOAD BIT(1) ^^^^^^^^^^ this part matches modparam name > #define USES_GUC_SUBMISSION (!!(i915_modparams.enable_guc & > BIT(USE_GUC_SUBMISSION_BIT))) > > Gah, that's so ugly. > > or > > static inline bool intel_use_guc_submission(void) "intel_" ? maybe correct prefix should be "i915_" ? > { > return i915_modparams.enable_guc & BIT(USE_GUC_SUBMISSION_BIT); > } I assume that above will still be wrapped inside macro #define USES_GUC_SUBMISSION(dev_priv) intel_uses_guc_submission() > > which at least stops being so shouty. > -Chris
Quoting Michal Wajdeczko (2017-12-01 14:09:31) > On Fri, 01 Dec 2017 13:36:06 +0100, Chris Wilson > <chris@chris-wilson.co.uk> wrote: > > > Quoting Michal Wajdeczko (2017-12-01 10:33:15) > >> We currently have two module parameters that control GuC: > >> "enable_guc_loading" and "enable_guc_submission". Whenever > >> we need submission=1, we also need loading=1. We also need > >> loading=1 when we want to want to load and verify the HuC. > >> > >> Lets combine above module parameters into one "enable_guc" > >> modparam. New supported bit values are: > >> > >> 0=disable GuC (no GuC submission, no HuC) > >> 1=enable GuC submission > >> 2=enable HuC load > >> > >> Special value "-1" can be used to let driver decide what > >> option should be enabled for given platform based on > >> hardware/firmware availability or preference. > >> > >> Explicit enabling any of the GuC features makes GuC load > >> a required step, fallback to non-GuC mode will not be > >> supported. > >> > >> v2: Don't use -EIO > >> > >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > >> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 5 +- > >> drivers/gpu/drm/i915/i915_params.c | 11 ++-- > >> drivers/gpu/drm/i915/i915_params.h | 3 +- > >> drivers/gpu/drm/i915/intel_uc.c | 100 > >> +++++++++++++++++++++---------------- > >> 4 files changed, 65 insertions(+), 54 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h > >> b/drivers/gpu/drm/i915/i915_drv.h > >> index 2c386c7..9162a60 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -3238,8 +3238,9 @@ intel_info(const struct drm_i915_private > >> *dev_priv) > >> #define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv)) > >> > >> /* Having a GuC is not the same as using a GuC */ > >> -#define USES_GUC(dev_priv) > >> (i915_modparams.enable_guc_loading) > >> -#define USES_GUC_SUBMISSION(dev_priv) > >> (i915_modparams.enable_guc_submission) > >> +#define USES_GUC(dev_priv) (!!(i915_modparams.enable_guc > > >> 0)) > >> +#define USES_GUC_SUBMISSION(dev_priv) (!!(i915_modparams.enable_guc & > >> 1)) > >> +#define USES_HUC(dev_priv) (!!(i915_modparams.enable_guc & > >> 2)) > > > > * channelling Joonas > > > > Please use a magic name for each bit and so > > > > #define USE_GUC_SUBMISSION_BIT 0 > > I was considering that, but then I decided to follow existing code > (see USES_PPGTT* and enable_ppgtt where we use plain numbers only) enable_ppgtt is on my list to kill. If vgpu didn't conspire against us, it would be simpler! > But if we want to start using magic values, then these values should > be defined in i915_params.h and rather in this way: > > #define ENABLE_GUC_SUBMISSION BIT(0) > #define ENABLE_GUC_HUC_LOAD BIT(1) > ^^^^^^^^^^ > this part matches modparam name Reasonable. > > #define USES_GUC_SUBMISSION (!!(i915_modparams.enable_guc & > > BIT(USE_GUC_SUBMISSION_BIT))) > > > > Gah, that's so ugly. > > > > or > > > > static inline bool intel_use_guc_submission(void) > > "intel_" ? maybe correct prefix should be "i915_" ? > > > { > > return i915_modparams.enable_guc & BIT(USE_GUC_SUBMISSION_BIT); > > } > > I assume that above will still be wrapped inside macro > > #define USES_GUC_SUBMISSION(dev_priv) intel_uses_guc_submission() Why? The compiler will dce functions just as well as macros; the age of the macro is long past, so the question is just a matter of how much is it worth continuing to cargo-cult a pattern that is long past requirement. Even its placement in i915_drv.h is up for debate. Maintaining the status quo is a valid argument, we just need a good reason to switch patterns (splitting up X-thousand lines of code into manageable chunks with consistent forward-facing interfaces is one such :). -Chris
On 12/1/2017 4:03 PM, Michal Wajdeczko wrote: > We currently have two module parameters that control GuC: > "enable_guc_loading" and "enable_guc_submission". Whenever > we need submission=1, we also need loading=1. We also need > loading=1 when we want to want to load and verify the HuC. > > Lets combine above module parameters into one "enable_guc" > modparam. New supported bit values are: > > 0=disable GuC (no GuC submission, no HuC) > 1=enable GuC submission > 2=enable HuC load > > Special value "-1" can be used to let driver decide what > option should be enabled for given platform based on > hardware/firmware availability or preference. > > Explicit enabling any of the GuC features makes GuC load > a required step, fallback to non-GuC mode will not be > supported. > > v2: Don't use -EIO > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> <snip> > +void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) > +{ > + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; > + struct intel_uc_fw *huc_fw = &dev_priv->huc.fw; > + int enable_guc = __get_default_enable_guc(dev_priv); > > /* A negative value means "use platform default" */ > - if (i915_modparams.enable_guc_submission < 0) > - i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv); > + if (i915_modparams.enable_guc < 0) > + i915_modparams.enable_guc = enable_guc; > + > + /* Verify GuC firmware availability */ > + if (USES_GUC(dev_priv) && !intel_uc_fw_is_selected(guc_fw)) { > + DRM_WARN("Incompatible option enable_guc=%d - %s\n", > + i915_modparams.enable_guc, > + !HAS_GUC(dev_priv) ? "no GuC hardware" : > + "no GuC firmware"); > + } > + > + /* Verify HuC firmware availability */ > + if (USES_GUC_SUBMISSION(dev_priv) && !intel_uc_fw_is_selected(huc_fw)) { should be USES_HUC here
On Fri, 01 Dec 2017 16:47:40 +0100, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > > > On 12/1/2017 4:03 PM, Michal Wajdeczko wrote: >> We currently have two module parameters that control GuC: >> "enable_guc_loading" and "enable_guc_submission". Whenever >> we need submission=1, we also need loading=1. We also need >> loading=1 when we want to want to load and verify the HuC. >> >> Lets combine above module parameters into one "enable_guc" >> modparam. New supported bit values are: >> >> 0=disable GuC (no GuC submission, no HuC) >> 1=enable GuC submission >> 2=enable HuC load >> >> Special value "-1" can be used to let driver decide what >> option should be enabled for given platform based on >> hardware/firmware availability or preference. >> >> Explicit enabling any of the GuC features makes GuC load >> a required step, fallback to non-GuC mode will not be >> supported. >> >> v2: Don't use -EIO >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > <snip> >> +void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) >> +{ >> + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; >> + struct intel_uc_fw *huc_fw = &dev_priv->huc.fw; >> + int enable_guc = __get_default_enable_guc(dev_priv); >> /* A negative value means "use platform default" */ >> - if (i915_modparams.enable_guc_submission < 0) >> - i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv); >> + if (i915_modparams.enable_guc < 0) >> + i915_modparams.enable_guc = enable_guc; >> + >> + /* Verify GuC firmware availability */ >> + if (USES_GUC(dev_priv) && !intel_uc_fw_is_selected(guc_fw)) { >> + DRM_WARN("Incompatible option enable_guc=%d - %s\n", >> + i915_modparams.enable_guc, >> + !HAS_GUC(dev_priv) ? "no GuC hardware" : >> + "no GuC firmware"); >> + } >> + >> + /* Verify HuC firmware availability */ >> + if (USES_GUC_SUBMISSION(dev_priv) && >> !intel_uc_fw_is_selected(huc_fw)) { > should be USES_HUC here good catch! thanks
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2c386c7..9162a60 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3238,8 +3238,9 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv)) /* Having a GuC is not the same as using a GuC */ -#define USES_GUC(dev_priv) (i915_modparams.enable_guc_loading) -#define USES_GUC_SUBMISSION(dev_priv) (i915_modparams.enable_guc_submission) +#define USES_GUC(dev_priv) (!!(i915_modparams.enable_guc > 0)) +#define USES_GUC_SUBMISSION(dev_priv) (!!(i915_modparams.enable_guc & 1)) +#define USES_HUC(dev_priv) (!!(i915_modparams.enable_guc & 2)) #define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 3328147..8654e45 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -154,13 +154,10 @@ i915_param_named_unsafe(edp_vswing, int, 0400, "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))"); -i915_param_named_unsafe(enable_guc_loading, int, 0400, - "Enable GuC firmware loading " - "(-1=auto, 0=never [default], 1=if available, 2=required)"); - -i915_param_named_unsafe(enable_guc_submission, int, 0400, - "Enable GuC submission " - "(-1=auto, 0=never [default], 1=if available, 2=required)"); +i915_param_named_unsafe(enable_guc, int, 0400, + "Enable GuC load for GuC submission and/or HuC load. " + "Required functionality can be selected using bitmask values. " + "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)"); i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level (-1:disabled (default), 0-3:enabled)"); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 8321bd8..10e2f74 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -42,8 +42,7 @@ param(int, disable_power_well, -1) \ param(int, enable_ips, 1) \ param(int, invert_brightness, 0) \ - param(int, enable_guc_loading, 0) \ - param(int, enable_guc_submission, 0) \ + param(int, enable_guc, 0) \ param(int, guc_log_level, -1) \ param(char *, guc_firmware_path, NULL) \ param(char *, huc_firmware_path, NULL) \ diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index ed2dd76..8a761af 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -47,35 +47,59 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv) return ret; } -void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) +static int __get_default_enable_guc(struct drm_i915_private *dev_priv) { - if (!HAS_GUC(dev_priv)) { - if (i915_modparams.enable_guc_loading > 0 || - i915_modparams.enable_guc_submission > 0) - DRM_INFO("Ignoring GuC options, no hardware\n"); + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; + struct intel_uc_fw *huc_fw = &dev_priv->huc.fw; + /* Default is to enable GuC/HuC if we know their firmwares */ + int enable_guc = (intel_uc_fw_is_selected(guc_fw) ? 1 : 0) | + (intel_uc_fw_is_selected(huc_fw) ? 2 : 0); - i915_modparams.enable_guc_loading = 0; - i915_modparams.enable_guc_submission = 0; - return; - } + /* Any platform specific fine-tuning can be done here */ - /* A negative value means "use platform default" */ - if (i915_modparams.enable_guc_loading < 0) - i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv); + /* Make sure that our "platform default" is well-defined */ + GEM_BUG_ON(enable_guc < 0); + GEM_BUG_ON((enable_guc > 0) && !intel_uc_fw_is_selected(guc_fw)); + GEM_BUG_ON((enable_guc & 2) && !intel_uc_fw_is_selected(huc_fw)); - /* Verify firmware version */ - if (i915_modparams.enable_guc_loading) { - if (!intel_uc_fw_is_selected(&dev_priv->guc.fw)) - i915_modparams.enable_guc_loading = 0; - } + return enable_guc; +} - /* Can't enable guc submission without guc loaded */ - if (!i915_modparams.enable_guc_loading) - i915_modparams.enable_guc_submission = 0; +/** + * intel_uc_sanitize_options - sanitize uC related modparam options + * @dev_priv: device private + * + * In case of "enable_guc" option this function will attempt to modify + * it only if it was initially set to "auto(-1)". Default value for this + * modparam varies between platforms and it is hardcoded in driver code. + * Any other modparam value is only monitored against availability of the + * related hardware or firmware definitions. + */ +void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) +{ + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; + struct intel_uc_fw *huc_fw = &dev_priv->huc.fw; + int enable_guc = __get_default_enable_guc(dev_priv); /* A negative value means "use platform default" */ - if (i915_modparams.enable_guc_submission < 0) - i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv); + if (i915_modparams.enable_guc < 0) + i915_modparams.enable_guc = enable_guc; + + /* Verify GuC firmware availability */ + if (USES_GUC(dev_priv) && !intel_uc_fw_is_selected(guc_fw)) { + DRM_WARN("Incompatible option enable_guc=%d - %s\n", + i915_modparams.enable_guc, + !HAS_GUC(dev_priv) ? "no GuC hardware" : + "no GuC firmware"); + } + + /* Verify HuC firmware availability */ + if (USES_GUC_SUBMISSION(dev_priv) && !intel_uc_fw_is_selected(huc_fw)) { + DRM_WARN("Incompatible option enable_guc=%d - %s\n", + i915_modparams.enable_guc, + !HAS_HUC(dev_priv) ? "no HuC hardware" : + "no HuC firmware"); + } } void intel_uc_init_early(struct drm_i915_private *dev_priv) @@ -155,6 +179,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) if (!USES_GUC(dev_priv)) return 0; + /* Late trap for incompatible modparam option */ + if (!HAS_GUC(dev_priv)) + return -ENODEV; + guc_disable_communication(guc); gen9_reset_guc_interrupts(dev_priv); @@ -229,12 +257,6 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) /* * We've failed to load the firmware :( - * - * Decide whether to disable GuC submission and fall back to - * execlist mode, and whether to hide the error by returning - * zero or to return -EIO, which the caller will treat as a - * nonfatal error (i.e. it doesn't prevent driver load, but - * marks the GPU as wedged until reset). */ err_interrupts: guc_disable_communication(guc); @@ -247,22 +269,14 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) err_guc: i915_ggtt_disable_guc(dev_priv); - if (i915_modparams.enable_guc_loading > 1 || - i915_modparams.enable_guc_submission > 1) { - DRM_ERROR("GuC init failed. Firmware loading disabled.\n"); - ret = -EIO; - } else { - DRM_NOTE("GuC init failed. Firmware loading disabled.\n"); - ret = 0; - } - - if (USES_GUC_SUBMISSION(dev_priv)) { - i915_modparams.enable_guc_submission = 0; - DRM_NOTE("Falling back from GuC submission to execlist mode\n"); - } - - i915_modparams.enable_guc_loading = 0; + dev_err(dev_priv->drm.dev, "GuC initialization failed.\n"); + /* + * There is no fallback as either user explicitly asked for the GuC + * or driver default was to run with GuC enabled. + */ + if (GEM_WARN_ON(ret == -EIO)) + return -EINVAL; return ret; }
We currently have two module parameters that control GuC: "enable_guc_loading" and "enable_guc_submission". Whenever we need submission=1, we also need loading=1. We also need loading=1 when we want to want to load and verify the HuC. Lets combine above module parameters into one "enable_guc" modparam. New supported bit values are: 0=disable GuC (no GuC submission, no HuC) 1=enable GuC submission 2=enable HuC load Special value "-1" can be used to let driver decide what option should be enabled for given platform based on hardware/firmware availability or preference. Explicit enabling any of the GuC features makes GuC load a required step, fallback to non-GuC mode will not be supported. v2: Don't use -EIO Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 5 +- drivers/gpu/drm/i915/i915_params.c | 11 ++-- drivers/gpu/drm/i915/i915_params.h | 3 +- drivers/gpu/drm/i915/intel_uc.c | 100 +++++++++++++++++++++---------------- 4 files changed, 65 insertions(+), 54 deletions(-)