[Intel-gfx] [PATCH v3 6/8] drm/i915/guc: Combine enable_guc_loading|submission modparams

[Chris Wilson]

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