| Message ID | 20161215154708.31521-6-arkadiusz.hiler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/12/2016 15:47, Arkadiusz Hiler wrote: > Currently guc_fw_path values can represent one of three possible states: > > 1) NULL - device without GuC > 2) '\0' - device with GuC but no known firmware > 3) else - device with GuC and known firmware > > Second case is used only to WARN at the later stage. > > We can WARN right away and merge cases 1 and 2 for later handling. > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Jeff McGee <jeff.mcgee@intel.com> > Cc: Michal Winiarski <michal.winiarski@intel.com> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 22 +++++++--------------- > 1 file changed, 7 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 0bb5fd1..075a103 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -460,12 +460,8 @@ int intel_guc_load(struct drm_i915_private *dev_priv) > intel_guc_fw_status_repr(guc_fw->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? */ > @@ -628,11 +624,9 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv, > void intel_guc_init(struct drm_i915_private *dev_priv) > { > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > - const char *fw_path; > + const char *fw_path = NULL; > > - if (!HAS_GUC_UCODE(dev_priv)) { > - fw_path = NULL; > - } else if (IS_SKYLAKE(dev_priv)) { > + if (IS_SKYLAKE(dev_priv)) { > fw_path = I915_SKL_GUC_UCODE; > guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR; > guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR; > @@ -644,24 +638,22 @@ void intel_guc_init(struct drm_i915_private *dev_priv) > fw_path = I915_KBL_GUC_UCODE; > guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR; > guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR; > - } else { > - fw_path = ""; /* unknown device */ > + } else if (HAS_GUC_UCODE(dev_priv)) { > + WARN(1, "No GuC firmware known for platform with GuC!\n"); I think you should also set i915.enable_guc_loading to zero here to avoid the later stages bothering with partial setup and cleanup. intel_uc_load specifically I think would with this patch run some setup then immediately fail and have to unwind. Regards, Tvrtko > } > > guc_fw->guc_fw_path = fw_path; > guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE; > guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE; > > - /* Early (and silent) return if GuC loading is disabled */ > + /* Early return if we do not have firmware to fetch */ > if (fw_path == NULL) > return; > - if (*fw_path == '\0') > - return; > > guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_PENDING; > DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path); > + > guc_fw_fetch(dev_priv, guc_fw); > - /* status must now be FAIL or SUCCESS */ > } > > /** >
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 0bb5fd1..075a103 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -460,12 +460,8 @@ int intel_guc_load(struct drm_i915_private *dev_priv) intel_guc_fw_status_repr(guc_fw->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? */ @@ -628,11 +624,9 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv, void intel_guc_init(struct drm_i915_private *dev_priv) { struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; - const char *fw_path; + const char *fw_path = NULL; - if (!HAS_GUC_UCODE(dev_priv)) { - fw_path = NULL; - } else if (IS_SKYLAKE(dev_priv)) { + if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_GUC_UCODE; guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR; guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR; @@ -644,24 +638,22 @@ void intel_guc_init(struct drm_i915_private *dev_priv) fw_path = I915_KBL_GUC_UCODE; guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR; guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR; - } else { - fw_path = ""; /* unknown device */ + } else if (HAS_GUC_UCODE(dev_priv)) { + WARN(1, "No GuC firmware known for platform with GuC!\n"); } guc_fw->guc_fw_path = fw_path; guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE; guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE; - /* Early (and silent) return if GuC loading is disabled */ + /* Early return if we do not have firmware to fetch */ if (fw_path == NULL) return; - if (*fw_path == '\0') - return; guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_PENDING; DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path); + guc_fw_fetch(dev_priv, guc_fw); - /* status must now be FAIL or SUCCESS */ } /**
Currently guc_fw_path values can represent one of three possible states: 1) NULL - device without GuC 2) '\0' - device with GuC but no known firmware 3) else - device with GuC and known firmware Second case is used only to WARN at the later stage. We can WARN right away and merge cases 1 and 2 for later handling. Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Michal Winiarski <michal.winiarski@intel.com> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> --- drivers/gpu/drm/i915/intel_guc_loader.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-)