| Message ID | 20171205163844.41264-6-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Michal Wajdeczko (2017-12-05 16:38:42) > -void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) > +static int __get_platform_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; > + int enable_guc = 0; > > - i915_modparams.enable_guc_loading = 0; > - i915_modparams.enable_guc_submission = 0; > - return; > - } > + /* Default is to enable GuC/HuC if we know their firmwares */ > + if (intel_uc_fw_is_selected(guc_fw)) > + enable_guc |= ENABLE_GUC_SUBMISSION; > + if (intel_uc_fw_is_selected(huc_fw)) > + enable_guc |= ENABLE_GUC_LOAD_HUC; > > - /* A negative value means "use platform default" */ > - if (i915_modparams.enable_guc_loading < 0) > - i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv); > + /* Any platform specific fine-tuning can be done here */ > > - /* 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; > > /* 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 = __get_platform_enable_guc(dev_priv); > + > + DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n", > + i915_modparams.enable_guc, > + yesno(intel_uc_is_using_guc_submission()), > + yesno(intel_uc_is_using_huc())); > + > + /* Verify GuC firmware availability */ > + if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) { > + DRM_WARN("Incompatible option detected: enable_guc=%d, %s!\n", > + i915_modparams.enable_guc, > + !HAS_GUC(dev_priv) ? "no GuC hardware" : > + "no GuC firmware"); > + } > + > + /* Verify HuC firmware availability */ > + if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) { > + DRM_WARN("Incompatible option detected: enable_guc=%d, %s!\n", > + i915_modparams.enable_guc, > + !HAS_HUC(dev_priv) ? "no HuC hardware" : > + "no HuC firmware"); > + } With the intent that after the warning, the uc load will fail and the module load will then abort? Just to be clear. > + > + /* Make sure that sanitization was done */ > + GEM_BUG_ON(i915_modparams.enable_guc < 0); > } > +static inline bool intel_uc_is_using_guc_submission(void) > +{ > + GEM_BUG_ON(i915_modparams.enable_guc < 0); > + return !!(i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION); Equivalent to a plain return i915_modparms.enable_guc & ENABLE_GUC_SUBMISSION thanks to the implicit (bool). -Chris
On Tue, 05 Dec 2017 23:47:21 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Michal Wajdeczko (2017-12-05 16:38:42) >> -void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) >> +static int __get_platform_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; >> + int enable_guc = 0; >> >> - i915_modparams.enable_guc_loading = 0; >> - i915_modparams.enable_guc_submission = 0; >> - return; >> - } >> + /* Default is to enable GuC/HuC if we know their firmwares */ >> + if (intel_uc_fw_is_selected(guc_fw)) >> + enable_guc |= ENABLE_GUC_SUBMISSION; >> + if (intel_uc_fw_is_selected(huc_fw)) >> + enable_guc |= ENABLE_GUC_LOAD_HUC; >> >> - /* A negative value means "use platform default" */ >> - if (i915_modparams.enable_guc_loading < 0) >> - i915_modparams.enable_guc_loading = >> HAS_GUC_UCODE(dev_priv); >> + /* Any platform specific fine-tuning can be done here */ >> >> - /* 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; >> >> /* 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 = >> __get_platform_enable_guc(dev_priv); >> + >> + DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n", >> + i915_modparams.enable_guc, >> + yesno(intel_uc_is_using_guc_submission()), >> + yesno(intel_uc_is_using_huc())); >> + >> + /* Verify GuC firmware availability */ >> + if (intel_uc_is_using_guc() && >> !intel_uc_fw_is_selected(guc_fw)) { >> + DRM_WARN("Incompatible option detected: enable_guc=%d, >> %s!\n", >> + i915_modparams.enable_guc, >> + !HAS_GUC(dev_priv) ? "no GuC hardware" : >> + "no GuC firmware"); >> + } >> + >> + /* Verify HuC firmware availability */ >> + if (intel_uc_is_using_huc() && >> !intel_uc_fw_is_selected(huc_fw)) { >> + DRM_WARN("Incompatible option detected: enable_guc=%d, >> %s!\n", >> + i915_modparams.enable_guc, >> + !HAS_HUC(dev_priv) ? "no HuC hardware" : >> + "no HuC firmware"); >> + } > > With the intent that after the warning, the uc load will fail and the > module load will then abort? Just to be clear. > Yes. As I'm not sure that we should ignore explicit user options and pretend that nothing really happen and continue with some fallback. But I must admit that with this patch we can upset users when module is loaded with enable_guc=-1(auto) and driver has corresponding GuC/HuC fw definitions but firmware blobs are not present on the target machine. Then we can wrongly read these options as explicit and abort module load due to fw upload failure. But impact of this issue can be minimized if we try to fetch GuC/HuC firmwares much earlier (init_early). Then we can quickly update our "preferred" auto options into "available". However, if we fail on any later stage, after we decided to enable GuC/Huc, there still will be no fallback and we will abort driver load. Michal
Quoting Michal Wajdeczko (2017-12-06 12:10:15) > On Tue, 05 Dec 2017 23:47:21 +0100, Chris Wilson > <chris@chris-wilson.co.uk> wrote: > > > Quoting Michal Wajdeczko (2017-12-05 16:38:42) > >> -void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) > >> +static int __get_platform_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; > >> + int enable_guc = 0; > >> > >> - i915_modparams.enable_guc_loading = 0; > >> - i915_modparams.enable_guc_submission = 0; > >> - return; > >> - } > >> + /* Default is to enable GuC/HuC if we know their firmwares */ > >> + if (intel_uc_fw_is_selected(guc_fw)) > >> + enable_guc |= ENABLE_GUC_SUBMISSION; > >> + if (intel_uc_fw_is_selected(huc_fw)) > >> + enable_guc |= ENABLE_GUC_LOAD_HUC; > >> > >> - /* A negative value means "use platform default" */ > >> - if (i915_modparams.enable_guc_loading < 0) > >> - i915_modparams.enable_guc_loading = > >> HAS_GUC_UCODE(dev_priv); > >> + /* Any platform specific fine-tuning can be done here */ > >> > >> - /* 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; > >> > >> /* 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 = > >> __get_platform_enable_guc(dev_priv); > >> + > >> + DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n", > >> + i915_modparams.enable_guc, > >> + yesno(intel_uc_is_using_guc_submission()), > >> + yesno(intel_uc_is_using_huc())); > >> + > >> + /* Verify GuC firmware availability */ > >> + if (intel_uc_is_using_guc() && > >> !intel_uc_fw_is_selected(guc_fw)) { > >> + DRM_WARN("Incompatible option detected: enable_guc=%d, > >> %s!\n", > >> + i915_modparams.enable_guc, > >> + !HAS_GUC(dev_priv) ? "no GuC hardware" : > >> + "no GuC firmware"); > >> + } > >> + > >> + /* Verify HuC firmware availability */ > >> + if (intel_uc_is_using_huc() && > >> !intel_uc_fw_is_selected(huc_fw)) { > >> + DRM_WARN("Incompatible option detected: enable_guc=%d, > >> %s!\n", > >> + i915_modparams.enable_guc, > >> + !HAS_HUC(dev_priv) ? "no HuC hardware" : > >> + "no HuC firmware"); > >> + } > > > > With the intent that after the warning, the uc load will fail and the > > module load will then abort? Just to be clear. > > > > Yes. As I'm not sure that we should ignore explicit user options and > pretend that nothing really happen and continue with some fallback. > > But I must admit that with this patch we can upset users when module is > loaded with enable_guc=-1(auto) and driver has corresponding GuC/HuC fw > definitions but firmware blobs are not present on the target machine. > > Then we can wrongly read these options as explicit and abort module load > due to fw upload failure. > > But impact of this issue can be minimized if we try to fetch GuC/HuC > firmwares much earlier (init_early). Then we can quickly update our > "preferred" auto options into "available". However, if we fail on any > later stage, after we decided to enable GuC/Huc, there still will be > no fallback and we will abort driver load. That worries me. But not enough to not give a r-b Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> I have nightmares of users bricking their computers just because an update goes wrong. Or more likely a kernel is updated without the accompanying linux-firmware. I think we should definitely be testing module-load with incorrect firmware and ensure we don't leave a system with a blank-screen-of-doom. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 937fa02..02551c7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3240,8 +3240,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) intel_uc_is_using_guc() +#define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission() +#define USES_HUC(dev_priv) intel_uc_is_using_huc() #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 7bc5386..8dfea03 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -147,13 +147,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 c48c88b..792ce26 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -25,8 +25,12 @@ #ifndef _I915_PARAMS_H_ #define _I915_PARAMS_H_ +#include <linux/bitops.h> #include <linux/cache.h> /* for __read_mostly */ +#define ENABLE_GUC_SUBMISSION BIT(0) +#define ENABLE_GUC_LOAD_HUC BIT(1) + #define I915_PARAMS_FOR_EACH(param) \ param(char *, vbt_firmware, NULL) \ param(int, modeset, -1) \ @@ -41,8 +45,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 c3981aa..7dfc7e0 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -47,35 +47,65 @@ 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_platform_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; + int enable_guc = 0; - i915_modparams.enable_guc_loading = 0; - i915_modparams.enable_guc_submission = 0; - return; - } + /* Default is to enable GuC/HuC if we know their firmwares */ + if (intel_uc_fw_is_selected(guc_fw)) + enable_guc |= ENABLE_GUC_SUBMISSION; + if (intel_uc_fw_is_selected(huc_fw)) + enable_guc |= ENABLE_GUC_LOAD_HUC; - /* A negative value means "use platform default" */ - if (i915_modparams.enable_guc_loading < 0) - i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv); + /* Any platform specific fine-tuning can be done here */ - /* 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; /* 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 = __get_platform_enable_guc(dev_priv); + + DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n", + i915_modparams.enable_guc, + yesno(intel_uc_is_using_guc_submission()), + yesno(intel_uc_is_using_huc())); + + /* Verify GuC firmware availability */ + if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) { + DRM_WARN("Incompatible option detected: enable_guc=%d, %s!\n", + i915_modparams.enable_guc, + !HAS_GUC(dev_priv) ? "no GuC hardware" : + "no GuC firmware"); + } + + /* Verify HuC firmware availability */ + if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) { + DRM_WARN("Incompatible option detected: enable_guc=%d, %s!\n", + i915_modparams.enable_guc, + !HAS_HUC(dev_priv) ? "no HuC hardware" : + "no HuC firmware"); + } + + /* Make sure that sanitization was done */ + GEM_BUG_ON(i915_modparams.enable_guc < 0); } void intel_uc_init_early(struct drm_i915_private *dev_priv) @@ -161,6 +191,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) if (!USES_GUC(dev_priv)) return 0; + if (!HAS_GUC(dev_priv)) { + ret = -ENODEV; + goto err_out; + } + guc_disable_communication(guc); gen9_reset_guc_interrupts(dev_priv); @@ -235,12 +270,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); @@ -252,23 +281,15 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) intel_guc_submission_fini(guc); err_guc: i915_ggtt_disable_guc(dev_priv); +err_out: + /* + * Note that there is no fallback as either user explicitly asked for + * the GuC or driver default option was to run with the GuC enabled. + */ + if (GEM_WARN_ON(ret == -EIO)) + ret = -EINVAL; - 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 %d\n", ret); return ret; } diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index e18d3bb..a995d60 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -26,6 +26,7 @@ #include "intel_guc.h" #include "intel_huc.h" +#include "i915_params.h" void intel_uc_sanitize_options(struct drm_i915_private *dev_priv); void intel_uc_init_early(struct drm_i915_private *dev_priv); @@ -35,4 +36,22 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv); int intel_uc_init_hw(struct drm_i915_private *dev_priv); void intel_uc_fini_hw(struct drm_i915_private *dev_priv); +static inline bool intel_uc_is_using_guc(void) +{ + GEM_BUG_ON(i915_modparams.enable_guc < 0); + return i915_modparams.enable_guc > 0; +} + +static inline bool intel_uc_is_using_guc_submission(void) +{ + GEM_BUG_ON(i915_modparams.enable_guc < 0); + return !!(i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION); +} + +static inline bool intel_uc_is_using_huc(void) +{ + GEM_BUG_ON(i915_modparams.enable_guc < 0); + return !!(i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC); +} + #endif
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 v3: define modparam bits (Chris) 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 | 7 ++- drivers/gpu/drm/i915/intel_uc.c | 109 ++++++++++++++++++++++--------------- drivers/gpu/drm/i915/intel_uc.h | 19 +++++++ 5 files changed, 96 insertions(+), 55 deletions(-)