[Intel-gfx] [PATCH] drm/i915/uc: Don't fail on HuC firmware failure

[Chris Wilson]

Quoting Michal Wajdeczko (2019-07-26 18:12:40)
> HuC is usually not a critical component, so we can safely ignore
> firmware load or authentication failures unless HuC was explicitly
> requested by the user.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c    | 8 ++++----
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 ++-
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 6 ++++++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 5b9b20d1cb6d..99419c5c0ad3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -472,7 +472,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
>  
>                 if (intel_uc_is_using_huc(uc)) {
>                         ret = intel_huc_fw_upload(huc);
> -                       if (ret)
> +                       if (ret && intel_uc_fw_is_overridden(&huc->fw))
>                                 goto err_out;
>                 }
>  
> @@ -494,9 +494,9 @@ int intel_uc_init_hw(struct intel_uc *uc)
>         if (ret)
>                 goto err_log_capture;
>  
> -       if (intel_uc_is_using_huc(uc)) {
> +       if (intel_uc_is_using_huc(uc) && intel_uc_fw_is_loaded(&huc->fw)) {

Can we even load the huc fw is !using_huc()?

>                 ret = intel_huc_auth(huc);
> -               if (ret)
> +               if (ret && intel_uc_fw_is_overridden(&huc->fw))
>                         goto err_communication;
>         }
>  
> @@ -515,7 +515,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
>         dev_info(i915->drm.dev, "GuC submission %s\n",
>                  enableddisabled(intel_uc_is_using_guc_submission(uc)));
>         dev_info(i915->drm.dev, "HuC %s\n",
> -                enableddisabled(intel_uc_is_using_huc(uc)));
> +                enableddisabled(intel_huc_is_authenticated(huc)));

Seems reasonable by itself.

>  
>         return 0;
>  
> 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 5fbdd17a864b..1e9ae38e5b10 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -146,7 +146,8 @@ __uc_fw_override(struct intel_uc_fw *uc_fw)
>                 break;
>         }
>  
> -       return uc_fw->path;
> +       uc_fw->user_overridden = uc_fw->path;

uc_fw->user_overridden = uc_fw->path && *uc_fw->path;

That is i915.huc_firmware="" should be a convenient way to disable
loading.

If we agree on that "creative misuse" of the modparam, or if you can
reassure me that it works correctly anyway,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris