| Message ID | 1518131035-24108-5-git-send-email-yaodong.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Jackie Li (2018-02-09 01:03:53) > On CNL A0 and Gen9, there's a hardware restriction that requires the > available GuC WOPCM size to be larger than or equal to HuC firmware size. > > This patch adds new verfication code to ensure the available GuC WOPCM size > to be larger than or equal to HuC firmware size on both Gen9 and CNL A0. > > v6: > - Extended HuC FW size check against GuC WOPCM size to all > Gen9 and CNL A0 platforms > > v7: > - Fixed patch format issues > > v8: > - Renamed variables and functions to avoid ambiguity (Joonas) > - Updated commit message and comments to be more comprehensive (Sagar) > > v9: > - Moved code that is not related to restriction check into a separate > patch and updated the commit message accordingly (Sagar/Michal) > - Avoided to call uc_get_fw_size for better layer isolation (Michal) > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: John Spotswood <john.a.spotswood@intel.com> > Cc: Jeff McGee <jeff.mcgee@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> (v8) > Signed-off-by: Jackie Li <yaodong.li@intel.com> <SNIP> > -static inline int gen9_guc_wopcm_size_check(struct intel_guc_wopcm *guc_wopcm) > +static inline int guc_wopcm_check_huc_fw_size(struct intel_guc_wopcm *guc_wopcm, > + u32 huc_fw_size) You can abbreviate here as there are local funcs, "check_huc_fw_fits" is enough. > +{ > + /* > + * On Gen9 & CNL A0, hardware requires the total available GuC WOPCM > + * size to be larger than or equal to HuC firmware size. Otherwise, > + * firmware uploading would fail. > + */ > + if (unlikely(guc_wopcm->size - GUC_WOPCM_RESERVED < huc_fw_size)) > + return -E2BIG; Again, it is overkill to try to educate branch predictor when this is called once during boot, just do the check without unlikely() wrapping :) > + > + return 0; > +} > + > +static inline int gen9_guc_wopcm_size_check(struct intel_guc_wopcm *guc_wopcm, > + u32 huc_fw_size) Abbreviate to gen9_check_dword_gap or something, don't bring the huc_fw_size here. > { > u32 guc_wopcm_start; > u32 delta; > @@ -58,16 +73,19 @@ static inline int gen9_guc_wopcm_size_check(struct intel_guc_wopcm *guc_wopcm) > if (unlikely(delta < sizeof(u32))) > return -E2BIG; > > - return 0; > + return guc_wopcm_check_huc_fw_size(guc_wopcm, huc_fw_size); > } > > -static inline int guc_wopcm_size_check(struct intel_guc *guc) > +static inline int guc_wopcm_size_check(struct intel_guc *guc, u32 huc_fw_size) > { > struct drm_i915_private *i915 = guc_to_i915(guc); > struct intel_guc_wopcm *guc_wopcm = &guc->wopcm; int err = 0; > > if (IS_GEN9(i915)) > - return gen9_guc_wopcm_size_check(guc_wopcm); > + return gen9_guc_wopcm_size_check(guc_wopcm, huc_fw_size); if (..) err = gen9_check_dword_gap(guc_wopcm); if (err) goto err; > + > + if (IS_CNL_REVID(i915, CNL_REVID_A0, CNL_REVID_A0)) > + return guc_wopcm_check_huc_fw_size(guc_wopcm, huc_fw_size); > if (IS_GEN9(i915) || IS_CNL_REVID(...)) err = check_huc_fw_fit(...) out: return err; This will better isolate the "two bugs", and huc_fw_size only goes to one of the checks, which makes the picture clearer. > return 0; > } > @@ -131,7 +149,7 @@ int intel_guc_wopcm_init(struct intel_guc_wopcm *guc_wopcm, u32 guc_fw_size, > guc->wopcm.top = top; > > /* Check platform specific restrictions */ > - err = guc_wopcm_size_check(guc); > + err = guc_wopcm_size_check(guc, huc_fw_size); s/guc_wopcm_size_check/size_checks/ as we're doing multiple (that's the only information the comment carries). Then drop the comment. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/intel_guc_wopcm.c b/drivers/gpu/drm/i915/intel_guc_wopcm.c index d0185b0..2e8e9ec 100644 --- a/drivers/gpu/drm/i915/intel_guc_wopcm.c +++ b/drivers/gpu/drm/i915/intel_guc_wopcm.c @@ -40,7 +40,22 @@ static inline u32 guc_wopcm_context_reserved_size(struct intel_guc *guc) return 0; } -static inline int gen9_guc_wopcm_size_check(struct intel_guc_wopcm *guc_wopcm) +static inline int guc_wopcm_check_huc_fw_size(struct intel_guc_wopcm *guc_wopcm, + u32 huc_fw_size) +{ + /* + * On Gen9 & CNL A0, hardware requires the total available GuC WOPCM + * size to be larger than or equal to HuC firmware size. Otherwise, + * firmware uploading would fail. + */ + if (unlikely(guc_wopcm->size - GUC_WOPCM_RESERVED < huc_fw_size)) + return -E2BIG; + + return 0; +} + +static inline int gen9_guc_wopcm_size_check(struct intel_guc_wopcm *guc_wopcm, + u32 huc_fw_size) { u32 guc_wopcm_start; u32 delta; @@ -58,16 +73,19 @@ static inline int gen9_guc_wopcm_size_check(struct intel_guc_wopcm *guc_wopcm) if (unlikely(delta < sizeof(u32))) return -E2BIG; - return 0; + return guc_wopcm_check_huc_fw_size(guc_wopcm, huc_fw_size); } -static inline int guc_wopcm_size_check(struct intel_guc *guc) +static inline int guc_wopcm_size_check(struct intel_guc *guc, u32 huc_fw_size) { struct drm_i915_private *i915 = guc_to_i915(guc); struct intel_guc_wopcm *guc_wopcm = &guc->wopcm; if (IS_GEN9(i915)) - return gen9_guc_wopcm_size_check(guc_wopcm); + return gen9_guc_wopcm_size_check(guc_wopcm, huc_fw_size); + + if (IS_CNL_REVID(i915, CNL_REVID_A0, CNL_REVID_A0)) + return guc_wopcm_check_huc_fw_size(guc_wopcm, huc_fw_size); return 0; } @@ -131,7 +149,7 @@ int intel_guc_wopcm_init(struct intel_guc_wopcm *guc_wopcm, u32 guc_fw_size, guc->wopcm.top = top; /* Check platform specific restrictions */ - err = guc_wopcm_size_check(guc); + err = guc_wopcm_size_check(guc, huc_fw_size); if (err) return err;