[PATCH v4 2/5] drm/i915/guc: symbolic names for GuC firmare loading preferences

Dave Gordon david.s.gordon at intel.com
Mon Aug 22 18:37:15 UTC 2016


The existing code that accesses the "enable_guc_loading"
parameter uses explicit numerical values for the various
possibilities,  including in one case relying on boolean
0/1 mapping to specific values (which could be confusing
for maintainers).

So this patch just provides and uses names for the values
representing the DEFAULT, DISABLED, PREFERRED, and MANDATORY
options that the user can select (-1, 0, 1, 2 respectively).

This should produce identical code to the previous version!

Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h        | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 13 +++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 941d70e..83c2f295 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -89,12 +89,26 @@ struct i915_guc_client {
 	uint64_t submissions[I915_NUM_ENGINES];
 };
 
+/*
+ * These signed ranges represent user-requested preferences.
+ * Out-of-range values from the user will be clipped towards
+ * zero: any negative value is treated as -1, anything over 2
+ * is just 2. ANY user-supplied value also taints the kernel.
+ */
 enum {
 	GUC_SUBMISSION_DEFAULT = -1,
 	GUC_SUBMISSION_DISABLED = 0,
 	GUC_SUBMISSION_PREFERRED,
 	GUC_SUBMISSION_MANDATORY
 };
+enum {
+	FIRMWARE_LOAD_DEFAULT = -1,
+	FIRMWARE_LOAD_DISABLED = 0,
+	FIRMWARE_LOAD_PREFERRED,
+	FIRMWARE_LOAD_MANDATORY
+};
+
+/* These represent the actual firmware status  */
 enum intel_guc_fw_status {
 	GUC_FIRMWARE_FAIL = -1,
 	GUC_FIRMWARE_NONE = 0,
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 49f846c..da047a8 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -438,7 +438,7 @@ int intel_guc_setup(struct drm_device *dev)
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
 	/* Loading forbidden, or no firmware to load? */
-	if (!i915.enable_guc_loading) {
+	if (i915.enable_guc_loading == FIRMWARE_LOAD_DISABLED) {
 		err = 0;
 		goto fail;
 	} else if (fw_path == NULL) {
@@ -533,7 +533,7 @@ int intel_guc_setup(struct drm_device *dev)
 	 * nonfatal error (i.e. it doesn't prevent driver load, but
 	 * marks the GPU as wedged until reset).
 	 */
-	if (i915.enable_guc_loading > 1) {
+	if (i915.enable_guc_loading >= FIRMWARE_LOAD_MANDATORY) {
 		ret = -EIO;
 	} else if (i915.enable_guc_submission >= GUC_SUBMISSION_MANDATORY) {
 		ret = -EIO;
@@ -699,9 +699,10 @@ void intel_guc_init(struct drm_device *dev)
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
 	const char *fw_path;
 
-	/* A negative value means "use platform default" */
-	if (i915.enable_guc_loading < 0)
-		i915.enable_guc_loading = HAS_GUC_UCODE(dev);
+	/* Any negative value means "use platform default" */
+	if (i915.enable_guc_loading <= FIRMWARE_LOAD_DEFAULT)
+		i915.enable_guc_loading = HAS_GUC_UCODE(dev) ?
+			FIRMWARE_LOAD_PREFERRED : FIRMWARE_LOAD_DISABLED;
 	if (i915.enable_guc_submission <= GUC_SUBMISSION_DEFAULT)
 		i915.enable_guc_submission = HAS_GUC_SCHED(dev) ?
 			GUC_SUBMISSION_PREFERRED : GUC_SUBMISSION_DISABLED;
@@ -730,7 +731,7 @@ void intel_guc_init(struct drm_device *dev)
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE;
 
 	/* Early (and silent) return if GuC loading is disabled */
-	if (!i915.enable_guc_loading)
+	if (i915.enable_guc_loading == FIRMWARE_LOAD_DISABLED)
 		return;
 	if (fw_path == NULL)
 		return;
-- 
1.9.1



More information about the Intel-gfx-trybot mailing list