There are 2 issues around handling of missing uC support: - We treat lack of uC HW and lack of uC FW definition as 2 different cases, but both of them mean that we don't support the uC on the platform we're running on. - We rely on the modparam to decide if we can take uC paths or not, but we don't sanitize it if it is set incorrectly on platform with no uC support. To fix both of them, unify the 2 cases in a single one and sanitize the modparam on invalid configuration (after printing an error message). The log has been adapted as well, since the user doesn't care why we don't support GuC/HuC (no HW or no FW), just that we do not. Developers can easily find the answer based on the platform, so we can simplify the log. Correcting the modparam has been preferred over failing the load since this is what we usually do for non-supported feature (e.g. the now gone enable_ppgtt would fall back to the highest supported PPGTT mode if the selected one was not available). Note that this patch purposely doesn't change the behavior for platforms that do have uC support, in which case we will still fail if enable_guc is set and the firmware is not available on the system. Suggested-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> --- drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 ++++--- drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 11 ++++--- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 37 ++++++++++++----------- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 ----- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 5 --- 5 files changed, 31 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c index 87169e826747..17ce78240cf8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c @@ -80,12 +80,10 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC); - if (!HAS_GT_UC(i915)) { - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; - return; - } + guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; + if (!HAS_GT_UC(i915)) + return; if (i915_modparams.guc_firmware_path) { guc_fw->path = i915_modparams.guc_firmware_path; @@ -112,6 +110,9 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR; guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR; } + + if (guc_fw->path) + guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; } /** diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c index ff6f7b157ecb..c3a7bd57fb55 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c @@ -74,12 +74,10 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC); - if (!HAS_GT_UC(dev_priv)) { - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; - return; - } + huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; + if (!HAS_GT_UC(dev_priv)) + return; if (i915_modparams.huc_firmware_path) { huc_fw->path = i915_modparams.huc_firmware_path; @@ -106,6 +104,9 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR; huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR; } + + if (huc_fw->path) + huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; } /** diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c index bdb171c3f36e..3f672ea7456b 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c @@ -68,7 +68,7 @@ static int __get_platform_enable_guc(struct intel_uc *uc) if (INTEL_GEN(uc_to_gt(uc)->i915) < 11) return 0; - if (intel_uc_fw_is_selected(guc_fw) && intel_uc_fw_is_selected(huc_fw)) + if (intel_uc_fw_supported(guc_fw) && intel_uc_fw_supported(huc_fw)) enable_guc |= ENABLE_GUC_LOAD_HUC; return enable_guc; @@ -123,26 +123,28 @@ static void sanitize_options_early(struct intel_uc *uc) yesno(intel_uc_is_using_huc(uc))); /* Verify GuC firmware availability */ - if (intel_uc_is_using_guc(uc) && !intel_uc_fw_is_selected(guc_fw)) { - DRM_WARN("Incompatible option detected: %s=%d, %s!\n", - "enable_guc", i915_modparams.enable_guc, - !intel_uc_fw_supported(guc_fw) ? - "no GuC hardware" : "no GuC firmware"); + if (intel_uc_is_using_guc(uc) && !intel_uc_fw_supported(guc_fw)) { + DRM_WARN("Incompatible option detected: enable_guc=%d, " + "but GuC is not supported!\n", + i915_modparams.enable_guc); + DRM_INFO("Disabling GuC/HuC loading!\n"); + i915_modparams.enable_guc = 0; } /* Verify HuC firmware availability */ - if (intel_uc_is_using_huc(uc) && !intel_uc_fw_is_selected(huc_fw)) { - DRM_WARN("Incompatible option detected: %s=%d, %s!\n", - "enable_guc", i915_modparams.enable_guc, - !intel_uc_fw_supported(huc_fw) ? - "no HuC hardware" : "no HuC firmware"); + if (intel_uc_is_using_huc(uc) && !intel_uc_fw_supported(huc_fw)) { + DRM_WARN("Incompatible option detected: enable_guc=%d, " + "but HuC is not supported!\n", + i915_modparams.enable_guc); + DRM_INFO("Disabling HuC loading!\n"); + i915_modparams.enable_guc &= ~ENABLE_GUC_LOAD_HUC; } /* XXX: GuC submission is unavailable for now */ if (intel_uc_is_using_guc_submission(uc)) { - DRM_INFO("Incompatible option detected: %s=%d, %s!\n", - "enable_guc", i915_modparams.enable_guc, - "GuC submission not supported"); + DRM_INFO("Incompatible option detected: enable_guc=%d, " + "but GuC submission is not supported!\n", + i915_modparams.enable_guc); DRM_INFO("Switching to non-GuC submission mode!\n"); i915_modparams.enable_guc &= ~ENABLE_GUC_SUBMISSION; } @@ -153,10 +155,9 @@ static void sanitize_options_early(struct intel_uc *uc) __get_default_guc_log_level(uc); if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc(uc)) { - DRM_WARN("Incompatible option detected: %s=%d, %s!\n", - "guc_log_level", i915_modparams.guc_log_level, - !intel_uc_fw_supported(guc_fw) ? - "no GuC hardware" : "GuC not enabled"); + DRM_WARN("Incompatible option detected: guc_log_level=%d, " + "but GuC is not enabled!\n", + i915_modparams.guc_log_level); i915_modparams.guc_log_level = 0; } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c index 8ce7210907c0..432b632b04c0 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -49,14 +49,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, GEM_BUG_ON(!intel_uc_fw_supported(uc_fw)); - if (!uc_fw->path) { - dev_info(dev_priv->drm.dev, - "%s: No firmware was defined for %s!\n", - intel_uc_fw_type_repr(uc_fw->type), - intel_platform_name(INTEL_INFO(dev_priv)->platform)); - return; - } - DRM_DEBUG_DRIVER("%s fw fetch %s\n", intel_uc_fw_type_repr(uc_fw->type), uc_fw->path); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h index 833d04d06576..55ac9eeab440 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h @@ -125,11 +125,6 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, uc_fw->type = type; } -static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw) -{ - return uc_fw->path != NULL; -} - static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw) { return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS; -- 2.22.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx