[Intel-gfx] [PATCH 1/3] drm/i915/uc: Consider enable_guc modparam during fw selection

[Chris Wilson]

Quoting Michal Wajdeczko (2019-07-30 19:19:01)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index fe3362fd7706..c8e5ad9807db 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -50,8 +50,7 @@ int intel_uc_resume(struct intel_uc *uc);
>  
>  static inline bool intel_uc_is_using_guc(struct intel_uc *uc)
>  {
> -       GEM_BUG_ON(i915_modparams.enable_guc < 0);
> -       return i915_modparams.enable_guc > 0;
> +       return intel_guc_is_supported(&uc->guc);

is_using_guc sounds like it should be looking at guc_is_running

I think the callers read better for me if I
s/intel_uc_is_using_guc/intel_uc_uses_guc/
or even better if intel_uc_supports_guc().

With that in mind,

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

> 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 ac91e3efd02b..3f051451caba 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -132,6 +132,27 @@ __uc_fw_auto_select(struct intel_uc_fw *uc_fw, enum intel_platform p, u8 rev)
>                         uc_fw->path = NULL;
>                 }
>         }
> +
> +       /* We don't want to enable GuC/HuC on pre-Gen11 by default */
> +       if ((i915_modparams.enable_guc < 0) && (p < INTEL_ICELAKE))
> +               uc_fw->path = NULL;

(Bonus) (brackets)
> +}
> +
> +static const char* __override_guc_firmware_path(void)
> +{
> +       /* XXX: don't check for GuC submission as it is unavailable for now */
> +       if ((i915_modparams.enable_guc < 0) ||
> +           (i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC))
> +               return i915_modparams.guc_firmware_path;
> +       return "";
> +}
> +
> +static const char* __override_huc_firmware_path(void)
> +{
> +       if ((i915_modparams.enable_guc < 0) ||
> +           (i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC))
> +               return i915_modparams.huc_firmware_path;

Looks habitual.

We can even lose the <0. No negative value other than -1 is documented.
-Chris