On 1/14/20 5:31 PM, Daniele Ceraolo Spurio wrote:
> Now that we can differentiate wants vs uses GuC/HuC, intel_uc_init is
> restricted to running only if we have successfully fetched the required
> blob(s) and are committed to using the microcontroller(s).
> The only remaining thing that can go wrong in uc_init is the allocation
> of GuC/HuC related objects; if we get such a failure better to bail out
> immediately instead of wedging later, like we do for e.g.
> intel_engines_init, since without objects we can't use the HW, including
> not being able to attempt the firmware load.
>
> While at it, remove the unneeded fw_cleanup call (this is handled
> outside of gt_init) and add a probe failure injection point for testing.
> Also, update the logs for <g/h>uc_init failures to probe_failure() since
> they will cause the driver load to fail.
Reviewed-by: Fernando Pacheco <fernando.pacheco@xxxxxxxxx>
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
> Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 4 +++-
> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 2 +-
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 24 +++++++++++++++++-------
> drivers/gpu/drm/i915/gt/uc/intel_uc.h | 4 ++--
> 5 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index da2b6e2ae692..85f21f18c785 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -584,7 +584,9 @@ int intel_gt_init(struct intel_gt *gt)
> if (err)
> goto err_engines;
>
> - intel_uc_init(>->uc);
> + err = intel_uc_init(>->uc);
> + if (err)
> + goto err_engines;
>
> err = intel_gt_resume(gt);
> if (err)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 5d00a3b2d914..c46f5ae77348 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -376,7 +376,7 @@ int intel_guc_init(struct intel_guc *guc)
> intel_uc_fw_fini(&guc->fw);
> err_fetch:
> intel_uc_fw_cleanup_fetch(&guc->fw);
> - DRM_DEV_DEBUG_DRIVER(gt->i915->drm.dev, "failed with %d\n", ret);
> + i915_probe_error(gt->i915, "failed with %d\n", ret);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 32a069841c14..5f448d0e360b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -127,7 +127,7 @@ int intel_huc_init(struct intel_huc *huc)
> intel_uc_fw_fini(&huc->fw);
> out:
> intel_uc_fw_cleanup_fetch(&huc->fw);
> - DRM_DEV_DEBUG_DRIVER(i915->drm.dev, "failed with %d\n", err);
> + i915_probe_error(i915, "failed with %d\n", err);
> return err;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 8843d4f16a7f..d57b731952ef 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -273,7 +273,7 @@ static void __uc_cleanup_firmwares(struct intel_uc *uc)
> intel_uc_fw_cleanup_fetch(&uc->guc.fw);
> }
>
> -static void __uc_init(struct intel_uc *uc)
> +static int __uc_init(struct intel_uc *uc)
> {
> struct intel_guc *guc = &uc->guc;
> struct intel_huc *huc = &uc->huc;
> @@ -282,19 +282,29 @@ static void __uc_init(struct intel_uc *uc)
> GEM_BUG_ON(!intel_uc_wants_guc(uc));
>
> if (!intel_uc_uses_guc(uc))
> - return;
> + return 0;
> +
> + if (i915_inject_probe_failure(uc_to_gt(uc)->i915))
> + return -ENOMEM;
>
> /* XXX: GuC submission is unavailable for now */
> GEM_BUG_ON(intel_uc_supports_guc_submission(uc));
>
> ret = intel_guc_init(guc);
> - if (ret) {
> - intel_uc_fw_cleanup_fetch(&huc->fw);
> - return;
> + if (ret)
> + return ret;
> +
> + if (intel_uc_uses_huc(uc)) {
> + ret = intel_huc_init(huc);
> + if (ret)
> + goto out_guc;
> }
>
> - if (intel_uc_uses_huc(uc))
> - intel_huc_init(huc);
> + return 0;
> +
> +out_guc:
> + intel_guc_fini(guc);
> + return ret;
> }
>
> static void __uc_fini(struct intel_uc *uc)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index f2f7351ff22a..2d9f17196761 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -16,7 +16,7 @@ struct intel_uc_ops {
> int (*sanitize)(struct intel_uc *uc);
> void (*init_fw)(struct intel_uc *uc);
> void (*fini_fw)(struct intel_uc *uc);
> - void (*init)(struct intel_uc *uc);
> + int (*init)(struct intel_uc *uc);
> void (*fini)(struct intel_uc *uc);
> int (*init_hw)(struct intel_uc *uc);
> void (*fini_hw)(struct intel_uc *uc);
> @@ -98,7 +98,7 @@ static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
> intel_uc_ops_function(sanitize, sanitize, int, 0);
> intel_uc_ops_function(fetch_firmwares, init_fw, void, );
> intel_uc_ops_function(cleanup_firmwares, fini_fw, void, );
> -intel_uc_ops_function(init, init, void, );
> +intel_uc_ops_function(init, init, int, 0);
> intel_uc_ops_function(fini, fini, void, );
> intel_uc_ops_function(init_hw, init_hw, int, 0);
> intel_uc_ops_function(fini_hw, fini_hw, void, );
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
