On Wed, Feb 22, 2017 at 01:41:25PM +0100, Arkadiusz Hiler wrote: > Currently fw->path values can represent one of three possible states: > > 1) NULL - device without the uC > 2) '\0' - device with the uC but have no firmware > 3) else - device with the uC and we have firmware > > Second case is used only to WARN at a later stage. > > We can WARN right away and merge cases 1 and 2. > > Code can be even further simplified and common (HuC/GuC logic) happening > right before the fetch can be offloaded to the common function. > > v2: fewer temporary variables, more straightforward flow (M. Wajdeczko) > v3: DRM_ERROR instead of WARN (M. Wajdeczko) > > Cc: Anusha Srivatsa <anusha.sriva...@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com> > Cc: Michal Winiarski <michal.winiar...@intel.com> > Cc: Michal Wajdeczko <michal.wajdec...@intel.com> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 39 > +++++++++++---------------------- > drivers/gpu/drm/i915/intel_huc.c | 20 +++++------------ > drivers/gpu/drm/i915/intel_uc.c | 5 +++-- > 3 files changed, 22 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 549a254..336e97d 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -433,12 +433,8 @@ int intel_guc_init_hw(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->load_status)); > > if (fw_path == NULL) { > - /* Device is known to have no uCode (e.g. no GuC) */ > + /* We do not have uCode for the device */ > return -ENXIO; > - } else if (*fw_path == '\0') { > - /* Device has a GuC but we don't know what f/w to load? */ > - WARN(1, "No GuC firmware known for this platform!\n"); > - return -ENODEV; > } > > /* Fetch failed, or already fetched but failed to load? */ > @@ -474,7 +470,6 @@ int intel_guc_init_hw(struct drm_i915_private *dev_priv) > return 0; > } > > - > /** > * intel_guc_fetch_fw() - determine and fetch firmware > * @guc: intel_guc struct > @@ -487,39 +482,31 @@ int intel_guc_init_hw(struct drm_i915_private *dev_priv) > void intel_guc_fetch_fw(struct intel_guc *guc) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > - const char *fw_path; > > - if (!HAS_GUC_UCODE(dev_priv)) { > - fw_path = NULL; > - } else if (IS_SKYLAKE(dev_priv)) { > - fw_path = I915_SKL_GUC_UCODE; > + guc->fw.path = NULL; > + guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; > + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; > + guc->fw.fw = INTEL_UC_FW_TYPE_GUC; > + > + if (IS_SKYLAKE(dev_priv)) { > + guc->fw.path = I915_SKL_GUC_UCODE; > guc->fw.major_ver_wanted = SKL_FW_MAJOR; > guc->fw.minor_ver_wanted = SKL_FW_MINOR; > } else if (IS_BROXTON(dev_priv)) { > - fw_path = I915_BXT_GUC_UCODE; > + guc->fw.path = I915_BXT_GUC_UCODE; > guc->fw.major_ver_wanted = BXT_FW_MAJOR; > guc->fw.minor_ver_wanted = BXT_FW_MINOR; > } else if (IS_KABYLAKE(dev_priv)) { > - fw_path = I915_KBL_GUC_UCODE; > + guc->fw.path = I915_KBL_GUC_UCODE; > guc->fw.major_ver_wanted = KBL_FW_MAJOR; > guc->fw.minor_ver_wanted = KBL_FW_MINOR; > } else { > - fw_path = ""; /* unknown device */ > + DRM_ERROR("No GuC firmware known for platform with GuC!\n"); > + i915.enable_guc_loading = 0; > + return;
Now plan for having fw_path overriden by a i915_param.guc_firmware. Perhaps something like if (i915_param.guc_firmware) { guc->fw.path = i915_param.guc_firmware; /* needs 0400! */ guc->fw.major_ver_wanted = -1; guc->fw.minor_ver_wanted = -1; } else if (IS_SKYLAKE.... works? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx