drm/i915: Fix handling of non-supported uC
authorDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Thu, 25 Jul 2019 00:18:07 +0000 (17:18 -0700)
committerChris Wilson <chris@chris-wilson.co.uk>
Thu, 25 Jul 2019 06:30:41 +0000 (07:30 +0100)
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@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190725001813.4740-3-daniele.ceraolospurio@intel.com
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
drivers/gpu/drm/i915/gt/uc/intel_uc.c
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h

index 87169e8..17ce782 100644 (file)
@@ -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;
 }
 
 /**
index ff6f7b1..c3a7bd5 100644 (file)
@@ -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;
 }
 
 /**
index bdb171c..3f672ea 100644 (file)
@@ -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;
        }
 
index 8ce7210..432b632 100644 (file)
@@ -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);
 
index 833d04d..55ac9ee 100644 (file)
@@ -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;