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 <[email protected]> > > Signed-off-by: Daniele Ceraolo Spurio <[email protected]> > Cc: Michal Wajdeczko <[email protected]> > Cc: John Harrison <[email protected]> > Cc: Matthew Brost <[email protected]> > --- > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
