Re: [PATCH v3 2/8] drm/i915: Fix handling of non-supported uC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On Thu, 25 Jul 2019 02:18:07 +0200, Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> wrote: 


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>


Note that more changes are planned around this code,
Reviewed-by: 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;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux