[Intel-gfx] [PATCH v2 1/2] drm/i915/guc : Removing enable_guc_loading module

Sujaritha Sundaresan sujaritha.sundaresan at intel.com
Fri Sep 8 21:47:31 UTC 2017


We currently have two module parameters that control GuC: "enable_guc_loading" and "enable_guc_submission".
Whenever we need i915.enable_guc_submission=1, we also need enable_guc_loading=1.
We also need enable_guc_loading=1 when we want to verify the HuC, which is every time we have a HuC (but all platforms with HuC have a GuC and viceversa).

v2: Unify seq_puts messages, correcting inconsistencies (Michal)
v2: Clarifying the commit message (Anusha)

Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
Cc: Oscar Mateo <oscar.mateo at intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 18 +++---
 drivers/gpu/drm/i915/i915_drv.c            |  4 +-
 drivers/gpu/drm/i915/i915_drv.h            |  1 +
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++-
 drivers/gpu/drm/i915/intel_guc_loader.c    | 50 ++---------------
 drivers/gpu/drm/i915/intel_guc_log.c       |  6 +-
 drivers/gpu/drm/i915/intel_huc.c           | 14 ++---
 drivers/gpu/drm/i915/intel_uc.c            | 90 ++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_uc.h            |  3 +-
 9 files changed, 97 insertions(+), 104 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7cc53c2..9396de0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1612,7 +1612,7 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 
 	if (!HAS_FBC(dev_priv)) {
-		seq_puts(m, "FBC unsupported on this chipset\n");
+		seq_puts(m, "not supported\n");
 		return 0;
 	}
 
@@ -1779,7 +1779,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 	unsigned int max_gpu_freq, min_gpu_freq;
 
 	if (!HAS_LLC(dev_priv)) {
-		seq_puts(m, "unsupported on this chipset\n");
+		seq_puts(m, "not supported\n");
 		return 0;
 	}
 
@@ -2339,7 +2339,7 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data)
 	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
 
 	if (!HAS_GUC(dev_priv)){
-		seq_puts(m, "No HuC support in HW\n");
+		seq_puts(m, "not supported\n");
 		return 0;
 	}
 		
@@ -2375,7 +2375,7 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
 	u32 tmp, i;
 
 	if (!HAS_GUC(dev_priv)){
-		seq_puts(m, "No GuC supprot in HW\n");
+		seq_puts(m, "not supported\n");
 		return 0;
 	}
 		
@@ -2476,7 +2476,7 @@ static bool check_guc_submission(struct seq_file *m)
 	const struct intel_guc *guc = &dev_priv->guc;
 
 	if (!guc->execbuf_client) {
-		seq_printf(m, "GuC submission %s\n",
+		seq_printf(m,
 			   HAS_GUC(dev_priv) ?
 			   "disabled" :
 			   "not supported");
@@ -2666,7 +2666,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 	bool enabled = false;
 
 	if (!HAS_PSR(dev_priv)) {
-		seq_puts(m, "PSR not supported\n");
+		seq_puts(m, "not supported\n");
 		return 0;
 	}
 
@@ -2819,7 +2819,7 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 
 	if (!HAS_RUNTIME_PM(dev_priv))
-		seq_puts(m, "Runtime power management not supported\n");
+		seq_puts(m, "not supported\n");
 
 	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
 	seq_printf(m, "IRQs disabled: %s\n",
@@ -3639,7 +3639,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
 		mutex_unlock(&drrs->mutex);
 	} else {
 		/* DRRS not supported. Print the VBT parameter*/
-		seq_puts(m, "\tDRRS Supported : No");
+		seq_puts(m, "not supported\n");
 	}
 	seq_puts(m, "\n");
 }
@@ -5039,4 +5039,4 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
 				    connector, &i915_panel_fops);
 
 	return 0;
-}
+}
\ No newline at end of file
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 00594dc..1c8d136 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1046,7 +1046,7 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 	i915.semaphores = intel_sanitize_semaphores(dev_priv, i915.semaphores);
 	DRM_DEBUG_DRIVER("use GPU semaphores? %s\n", yesno(i915.semaphores));
 
-	intel_guc_sanitize_submission(dev_priv);
+	intel_uc_sanitize_options(dev_priv);
 
 	intel_gvt_sanitize_options(dev_priv);
 }
@@ -2769,4 +2769,4 @@ static int intel_runtime_resume(struct device *kdev)
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_drm.c"
-#endif
+#endif
\ No newline at end of file
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 437c3c6..424cd78 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3068,6 +3068,7 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
 #define HAS_GUC(dev_priv)			((dev_priv)->info.has_guc)
 #define HAS_GUC_UCODE(dev_priv)		((dev_priv)->guc.fw.path != NULL)
 #define HAS_HUC_UCODE(dev_priv)		((dev_priv)->huc.fw.path != NULL)
+
 #define NEEDS_GUC_LOADING(dev_priv) \
 	(HAS_GUC(dev_priv) && \
 	(i915.enable_guc_submission || HAS_HUC_UCODE(dev_priv)))
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 639e0d8..a15146e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1109,12 +1109,17 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 
 	vaddr = i915_gem_object_pin_map(guc->stage_desc_pool->obj, I915_MAP_WB);
 	if (IS_ERR(vaddr)) {
-		i915_vma_unpin_and_release(&guc->stage_desc_pool);
-		return PTR_ERR(vaddr);
+		ret = PTR_ERR(vaddr);
+		goto err_vma;
 	}
 
 	guc->stage_desc_pool_vaddr = vaddr;
 
+	ret = intel_guc_log_create(guc);
+	if (ret < 0)
+		goto err_vaddr;
+	
+
 	ret = guc_ads_create(guc);
 	if (ret < 0)
 		goto err_log;
@@ -1125,6 +1130,11 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 
 err_log:
 	intel_guc_log_destroy(guc);
+	
+err_vaddr:
+	i915_gem_object_unpin_map(guc->stage_desc_pool->obj);
+err_vma:
+	i915_vma_unpin_and_release(&guc->stage_desc_pool);
 	return ret;
 }
 
@@ -1134,6 +1144,7 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
 
 	ida_destroy(&guc->stage_ids);
 	guc_ads_destroy(guc);
+	intel_guc_log_destroy(guc);
 	i915_gem_object_unpin_map(guc->stage_desc_pool->obj);
 	i915_vma_unpin_and_release(&guc->stage_desc_pool);
 }
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 673d67a..672c83c 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -375,44 +375,6 @@ int intel_guc_init_hw(struct intel_guc *guc)
 	return 0;
 }
 
-void intel_guc_sanitize_submission(struct drm_i915_private *dev_priv)
-{
-	/* Verify hardware support */
-	if (!HAS_GUC(dev_priv)){
-		if (i915.enable_guc_submission > 0)
-				DRM_INFO("Ignoring GuC submission enable, no HW\n");
-		i915.enable_guc_submission = 0;
-		return;
-	}
-
-	/* Verify firmware support */
-	if (!HAS_GUC_UCODE(dev_priv)){
-		if (i915.enable_guc_submission == 1){
-			DRM_INFO("Ignoring GuC submission enable, no FW\n");
-			i915.enable_guc_submission = 0;
-			return;
-		}
-
-		if (i915.enable_guc_submission < 0){
-			i915.enable_guc_submission = 0;
-			return;
-		}
-
-		/* 
-		 * If "required"(> 1), let it continue and we will fail later 
-		 * due to lack of firmware
-		 */
-	}
-
-	/*
-	 * A negative value means "use platform default" (enabled if we have
-	 * survived to get here)
-	 */
-	if (i915.enable_guc_submission < 0)
-		i915.enable_guc_submission = 1;
-}
-
-
 /**
  * intel_guc_select_fw() - selects GuC firmware for loading
  * @guc:	intel_guc struct
@@ -426,7 +388,11 @@ void intel_guc_select_fw(struct intel_guc *guc)
 	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 	guc->fw.type = INTEL_UC_FW_TYPE_GUC;
 
-	if (i915.guc_firmware_path) {
+	
+	if (HAS_GUC(dev_priv)){
+			DRM_ERROR("No GuC FW known for a platform with GuC!\n");
+		return;
+	} else if (i915.guc_firmware_path) {
 		guc->fw.path = i915.guc_firmware_path;
 		guc->fw.major_ver_wanted = 0;
 		guc->fw.minor_ver_wanted = 0;
@@ -446,10 +412,6 @@ void intel_guc_select_fw(struct intel_guc *guc)
 		guc->fw.path = I915_GLK_GUC_UCODE;
 		guc->fw.major_ver_wanted = GLK_FW_MAJOR;
 		guc->fw.minor_ver_wanted = GLK_FW_MINOR;
-	} else {
-		if (HAS_GUC(dev_priv))
-			DRM_ERROR("No GuC FW known for a platform with GuC!\n");
-		return;
-	}
+	} 
 
 }
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 592b449..16d3b87 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -502,7 +502,7 @@ static void guc_flush_logs(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	if (HAS_GUC_UCODE(dev_priv) || (i915.guc_log_level < 0))
+	if (!i915.enable_guc_submission || (i915.guc_log_level < 0))
 		return;
 
 	/* First disable the interrupts, will be renabled afterwards */
@@ -641,7 +641,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 
 void i915_guc_log_register(struct drm_i915_private *dev_priv)
 {
-	if (HAS_GUC_UCODE(dev_priv) || i915.guc_log_level < 0)
+	if (!i915.enable_guc_submission || i915.guc_log_level < 0)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
@@ -651,7 +651,7 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
 
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 {
-	if (HAS_GUC_UCODE(dev_priv))
+	if (!i915.enable_guc_submission)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index bf831a77..57dce67 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -155,7 +155,12 @@ void intel_huc_select_fw(struct intel_huc *huc)
 	huc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 	huc->fw.type = INTEL_UC_FW_TYPE_HUC;
 
-	if (i915.huc_firmware_path) {
+	
+		/* For now, everything with a GuC also has a HuC */
+	if (HAS_GUC(dev_priv)){
+			DRM_ERROR("No HuC FW known for a platform with HuC!\n");
+		return;
+	} else if (i915.huc_firmware_path) {
 		huc->fw.path = i915.huc_firmware_path;
 		huc->fw.major_ver_wanted = 0;
 		huc->fw.minor_ver_wanted = 0;
@@ -175,12 +180,7 @@ void intel_huc_select_fw(struct intel_huc *huc)
 		huc->fw.path = I915_GLK_HUC_UCODE;
 		huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
 		huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
-	} else {
-		/* For now, everything with a GuC also has a HuC */
-		if (HAS_GUC(dev_priv))
-			DRM_ERROR("No HuC FW known for a platform with HuC!\n");
-		return;
-	}
+	} 
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 44faeac..52687e3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -60,24 +60,49 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-static void guc_write_irq_trigger(struct intel_guc *guc)
+void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	/* Verify hardware support */
+	if (!HAS_GUC(dev_priv)){
+		if (i915.enable_guc_submission > 0)
+				DRM_INFO("Ignoring GuC submission enable, no HW\n");
+		i915.enable_guc_submission = 0;
+		return;
+	}
 
-	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
+	/* Verify firmware support */
+	if (!HAS_GUC_UCODE(dev_priv)){
+		if (i915.enable_guc_submission == 1){
+			DRM_INFO("Ignoring GuC submission enable, no FW\n");
+			i915.enable_guc_submission = 0;
+			return;
+		}
+
+		if (i915.enable_guc_submission < 0){
+			i915.enable_guc_submission = 0;
+			return;
+		}
+
+		/* 
+		 * If "required"(> 1), let it continue and we will fail later 
+		 * due to lack of firmware
+		 */
+	}
+
+	/*
+	 * A negative value means "use platform default" (enabled if we have
+	 * survived to get here)
+	 */
+	if (i915.enable_guc_submission < 0)
+		i915.enable_guc_submission = 1;
 }
 
-void intel_uc_init_early(struct drm_i915_private *dev_priv)
-{
-	struct intel_guc *guc = &dev_priv->guc;
 
-	intel_guc_ct_init_early(&guc->ct);
-	intel_guc_select_fw(&dev_priv->guc);
-	intel_huc_select_fw(&dev_priv->huc);
+static void guc_write_irq_trigger(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	mutex_init(&guc->send_mutex);
-	guc->send = intel_guc_send_nop;
-	guc->notify = guc_write_irq_trigger;
+	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
 }
 
 static void fetch_uc_fw(struct drm_i915_private *dev_priv,
@@ -218,6 +243,12 @@ static void fetch_uc_fw(struct drm_i915_private *dev_priv,
 	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
 }
 
+void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
+{
+	__intel_uc_fw_fini(&dev_priv->guc.fw);
+	__intel_uc_fw_fini(&dev_priv->huc.fw);
+}
+
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
 	if(!HAS_GUC(dev_priv))
@@ -226,10 +257,17 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 	fetch_uc_fw(dev_priv, &dev_priv->guc.fw);
 }
 
-void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
+void intel_uc_init_early(struct drm_i915_private *dev_priv)
 {
-	__intel_uc_fw_fini(&dev_priv->guc.fw);
-	__intel_uc_fw_fini(&dev_priv->huc.fw);
+	struct intel_guc *guc = &dev_priv->guc;
+
+	intel_guc_ct_init_early(&guc->ct);
+	intel_guc_select_fw(&dev_priv->guc);
+	intel_huc_select_fw(&dev_priv->huc);
+
+	mutex_init(&guc->send_mutex);
+	guc->send = intel_guc_send_nop;
+	guc->notify = guc_write_irq_trigger;
 }
 
 static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
@@ -298,20 +336,6 @@ static void guc_disable_communication(struct intel_guc *guc)
 	guc->send = intel_guc_send_nop;
 }
 
-static int guc_shared_objects_init(struct intel_guc *guc)
-{
-	int ret;
-
-	ret = intel_guc_log_create(guc);
-	if (ret < 0)
-		return ret;
-}
-
-static void guc_shared_objects_fini(struct intel_guc *guc)
-{
-	intel_guc_log_destroy(guc);
-}
-
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
@@ -325,9 +349,6 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 	/* We need to notify the guc whenever we change the GGTT */
 	i915_ggtt_enable_guc(dev_priv);
-	ret = guc_shared_objects_init(guc);
-	if (ret)
-		goto err_guc;
 
 	if (i915.enable_guc_submission) {
 		/*
@@ -336,7 +357,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 		 */
 		ret = i915_guc_submission_init(dev_priv);
 		if (ret)
-			goto err_shared;
+			goto err_guc;
 	}
 
 	/* init WOPCM */
@@ -406,8 +427,6 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 err_submission:
 	if (i915.enable_guc_submission)
 		i915_guc_submission_fini(dev_priv);
-err_shared:
-	guc_shared_objects_fini(guc);
 err_guc:
 	i915_ggtt_disable_guc(dev_priv);
 
@@ -443,7 +462,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 		i915_guc_submission_fini(dev_priv);
 	}
 
-	guc_shared_objects_fini(&dev_priv->guc);
 	i915_ggtt_disable_guc(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index ea2a32b..731ec16 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -220,6 +220,7 @@ struct intel_huc {
 };
 
 /* intel_uc.c */
+void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
@@ -246,8 +247,8 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 int intel_guc_resume(struct drm_i915_private *dev_priv);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
+
 /* i915_guc_submission.c */
-void intel_guc_sanitize_submission(struct drm_i915_private *dev_priv);
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
 int i915_guc_wq_reserve(struct drm_i915_gem_request *rq);
-- 
1.9.1



More information about the Intel-gfx mailing list