diff mbox

[v3,2/3] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init

Message ID 20180628141522.62788-2-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Wajdeczko June 28, 2018, 2:15 p.m. UTC 
We're fetching GuC/HuC firmwares directly from uc level during
init_early stage but this breaks guc/huc struct isolation and
also strict SW-only initialization rule for init_early. Move fw
fetching to init phase and do it separately per guc/huc struct.

v2: don't forget to move wopcm_init - Michele
v3: fetch in init_misc phase - Michal

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> #2
---
 drivers/gpu/drm/i915/i915_gem.c  |  7 ++++---
 drivers/gpu/drm/i915/intel_guc.c |  9 ++++++++-
 drivers/gpu/drm/i915/intel_huc.c |  8 ++++++++
 drivers/gpu/drm/i915/intel_huc.h |  6 ++++++
 drivers/gpu/drm/i915/intel_uc.c  | 28 +++++++++++++++-------------
 5 files changed, 41 insertions(+), 17 deletions(-)

Comments

Michel Thierry June 28, 2018, 9:56 p.m. UTC | #1 
On 6/28/2018 7:15 AM, Michal Wajdeczko wrote:
> We're fetching GuC/HuC firmwares directly from uc level during
> init_early stage but this breaks guc/huc struct isolation and
> also strict SW-only initialization rule for init_early. Move fw
> fetching to init phase and do it separately per guc/huc struct.
> 
> v2: don't forget to move wopcm_init - Michele
> v3: fetch in init_misc phase - Michal
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #2

R-b stands for v3

> ---
>   drivers/gpu/drm/i915/i915_gem.c  |  7 ++++---
>   drivers/gpu/drm/i915/intel_guc.c |  9 ++++++++-
>   drivers/gpu/drm/i915/intel_huc.c |  8 ++++++++
>   drivers/gpu/drm/i915/intel_huc.h |  6 ++++++
>   drivers/gpu/drm/i915/intel_uc.c  | 28 +++++++++++++++-------------
>   5 files changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5a9cae6..8954db6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5459,13 +5459,13 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>   	if (ret)
>   		return ret;
>   
> -	ret = intel_wopcm_init(&dev_priv->wopcm);
> +	ret = intel_uc_init_misc(dev_priv);
>   	if (ret)
>   		return ret;
>   
> -	ret = intel_uc_init_misc(dev_priv);
> +	ret = intel_wopcm_init(&dev_priv->wopcm);
>   	if (ret)
> -		return ret;
> +		goto err_uc_misc;
>   
>   	/* This is just a security blanket to placate dragons.
>   	 * On some systems, we very sporadically observe that the first TLBs
> @@ -5563,6 +5563,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   	mutex_unlock(&dev_priv->drm.struct_mutex);
>   
> +err_uc_misc:
>   	intel_uc_fini_misc(dev_priv);
>   
>   	if (ret != -EIO)
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 0b06f27..53b43bc 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -139,6 +139,7 @@ static void guc_fini_wq(struct intel_guc *guc)
>   
>   int intel_guc_init_misc(struct intel_guc *guc)
>   {
> +	struct drm_i915_private *i915 = guc_to_i915(guc);
>   	int ret;
>   
>   	guc_init_ggtt_pin_bias(guc);
> @@ -147,11 +148,14 @@ int intel_guc_init_misc(struct intel_guc *guc)
>   	if (ret)
>   		return ret;
>   
> +	intel_uc_fw_fetch(i915, &guc->fw);
> +
>   	return 0;
>   }
>   
>   void intel_guc_fini_misc(struct intel_guc *guc)
>   {
> +	intel_uc_fw_fini(&guc->fw);
>   	guc_fini_wq(guc);
>   }
>   
> @@ -189,7 +193,7 @@ int intel_guc_init(struct intel_guc *guc)
>   
>   	ret = guc_shared_data_create(guc);
>   	if (ret)
> -		return ret;
> +		goto err_fetch;
>   	GEM_BUG_ON(!guc->shared_data);
>   
>   	ret = intel_guc_log_create(&guc->log);
> @@ -210,6 +214,8 @@ int intel_guc_init(struct intel_guc *guc)
>   	intel_guc_log_destroy(&guc->log);
>   err_shared:
>   	guc_shared_data_destroy(guc);
> +err_fetch:
> +	intel_uc_fw_fini(&guc->fw);
>   	return ret;
>   }
>   
> @@ -221,6 +227,7 @@ void intel_guc_fini(struct intel_guc *guc)
>   	intel_guc_ads_destroy(guc);
>   	intel_guc_log_destroy(&guc->log);
>   	guc_shared_data_destroy(guc);
> +	intel_uc_fw_fini(&guc->fw);
>   }
>   
>   static u32 guc_ctl_debug_flags(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 2912852..ffcad5f 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -32,6 +32,14 @@ void intel_huc_init_early(struct intel_huc *huc)
>   	intel_huc_fw_init_early(huc);
>   }
>   
> +int intel_huc_init_misc(struct intel_huc *huc)
> +{
> +	struct drm_i915_private *i915 = huc_to_i915(huc);
> +
> +	intel_uc_fw_fetch(i915, &huc->fw);
> +	return 0;
> +}
> +
>   /**
>    * intel_huc_auth() - Authenticate HuC uCode
>    * @huc: intel_huc structure
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index aa85490..7e41d87 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -36,9 +36,15 @@ struct intel_huc {
>   };
>   
>   void intel_huc_init_early(struct intel_huc *huc);
> +int intel_huc_init_misc(struct intel_huc *huc);
>   int intel_huc_auth(struct intel_huc *huc);
>   int intel_huc_check_status(struct intel_huc *huc);
>   
> +static inline void intel_huc_fini_misc(struct intel_huc *huc)
> +{
> +	intel_uc_fw_fini(&huc->fw);
> +}
> +
>   static inline int intel_huc_sanitize(struct intel_huc *huc)
>   {
>   	intel_uc_fw_sanitize(&huc->fw);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index cd49b4f..7c95697 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -171,24 +171,11 @@ void intel_uc_init_early(struct drm_i915_private *i915)
>   	intel_huc_init_early(huc);
>   
>   	sanitize_options_early(i915);
> -
> -	if (USES_GUC(i915))
> -		intel_uc_fw_fetch(i915, &guc->fw);
> -
> -	if (USES_HUC(i915))
> -		intel_uc_fw_fetch(i915, &huc->fw);
>   }
>   
>   void intel_uc_cleanup_early(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> -	struct intel_huc *huc = &i915->huc;
> -
> -	if (USES_HUC(i915))
> -		intel_uc_fw_fini(&huc->fw);
> -
> -	if (USES_GUC(i915))
> -		intel_uc_fw_fini(&guc->fw);
>   
>   	guc_free_load_err_log(guc);
>   }
> @@ -252,6 +239,7 @@ static void guc_disable_communication(struct intel_guc *guc)
>   int intel_uc_init_misc(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> +	struct intel_huc *huc = &i915->huc;
>   	int ret;
>   
>   	if (!USES_GUC(i915))
> @@ -261,16 +249,30 @@ int intel_uc_init_misc(struct drm_i915_private *i915)
>   	if (ret)
>   		return ret;
>   
> +	if (USES_HUC(i915)) {
> +		ret = intel_huc_init_misc(huc);
> +		if (ret)
> +			goto err_guc;
> +	}
> +
>   	return 0;
> +
> +err_guc:
> +	intel_guc_fini_misc(guc);
> +	return ret;
>   }
>   
>   void intel_uc_fini_misc(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> +	struct intel_huc *huc = &i915->huc;
>   
>   	if (!USES_GUC(i915))
>   		return;
>   
> +	if (USES_HUC(i915))
> +		intel_huc_fini_misc(huc);
> +
>   	intel_guc_fini_misc(guc);
>   }
>   
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5a9cae6..8954db6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5459,13 +5459,13 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	ret = intel_wopcm_init(&dev_priv->wopcm);
+	ret = intel_uc_init_misc(dev_priv);
 	if (ret)
 		return ret;
 
-	ret = intel_uc_init_misc(dev_priv);
+	ret = intel_wopcm_init(&dev_priv->wopcm);
 	if (ret)
-		return ret;
+		goto err_uc_misc;
 
 	/* This is just a security blanket to placate dragons.
 	 * On some systems, we very sporadically observe that the first TLBs
@@ -5563,6 +5563,7 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
+err_uc_misc:
 	intel_uc_fini_misc(dev_priv);
 
 	if (ret != -EIO)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 0b06f27..53b43bc 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -139,6 +139,7 @@  static void guc_fini_wq(struct intel_guc *guc)
 
 int intel_guc_init_misc(struct intel_guc *guc)
 {
+	struct drm_i915_private *i915 = guc_to_i915(guc);
 	int ret;
 
 	guc_init_ggtt_pin_bias(guc);
@@ -147,11 +148,14 @@  int intel_guc_init_misc(struct intel_guc *guc)
 	if (ret)
 		return ret;
 
+	intel_uc_fw_fetch(i915, &guc->fw);
+
 	return 0;
 }
 
 void intel_guc_fini_misc(struct intel_guc *guc)
 {
+	intel_uc_fw_fini(&guc->fw);
 	guc_fini_wq(guc);
 }
 
@@ -189,7 +193,7 @@  int intel_guc_init(struct intel_guc *guc)
 
 	ret = guc_shared_data_create(guc);
 	if (ret)
-		return ret;
+		goto err_fetch;
 	GEM_BUG_ON(!guc->shared_data);
 
 	ret = intel_guc_log_create(&guc->log);
@@ -210,6 +214,8 @@  int intel_guc_init(struct intel_guc *guc)
 	intel_guc_log_destroy(&guc->log);
 err_shared:
 	guc_shared_data_destroy(guc);
+err_fetch:
+	intel_uc_fw_fini(&guc->fw);
 	return ret;
 }
 
@@ -221,6 +227,7 @@  void intel_guc_fini(struct intel_guc *guc)
 	intel_guc_ads_destroy(guc);
 	intel_guc_log_destroy(&guc->log);
 	guc_shared_data_destroy(guc);
+	intel_uc_fw_fini(&guc->fw);
 }
 
 static u32 guc_ctl_debug_flags(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 2912852..ffcad5f 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -32,6 +32,14 @@  void intel_huc_init_early(struct intel_huc *huc)
 	intel_huc_fw_init_early(huc);
 }
 
+int intel_huc_init_misc(struct intel_huc *huc)
+{
+	struct drm_i915_private *i915 = huc_to_i915(huc);
+
+	intel_uc_fw_fetch(i915, &huc->fw);
+	return 0;
+}
+
 /**
  * intel_huc_auth() - Authenticate HuC uCode
  * @huc: intel_huc structure
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index aa85490..7e41d87 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -36,9 +36,15 @@  struct intel_huc {
 };
 
 void intel_huc_init_early(struct intel_huc *huc);
+int intel_huc_init_misc(struct intel_huc *huc);
 int intel_huc_auth(struct intel_huc *huc);
 int intel_huc_check_status(struct intel_huc *huc);
 
+static inline void intel_huc_fini_misc(struct intel_huc *huc)
+{
+	intel_uc_fw_fini(&huc->fw);
+}
+
 static inline int intel_huc_sanitize(struct intel_huc *huc)
 {
 	intel_uc_fw_sanitize(&huc->fw);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index cd49b4f..7c95697 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -171,24 +171,11 @@  void intel_uc_init_early(struct drm_i915_private *i915)
 	intel_huc_init_early(huc);
 
 	sanitize_options_early(i915);
-
-	if (USES_GUC(i915))
-		intel_uc_fw_fetch(i915, &guc->fw);
-
-	if (USES_HUC(i915))
-		intel_uc_fw_fetch(i915, &huc->fw);
 }
 
 void intel_uc_cleanup_early(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
-	struct intel_huc *huc = &i915->huc;
-
-	if (USES_HUC(i915))
-		intel_uc_fw_fini(&huc->fw);
-
-	if (USES_GUC(i915))
-		intel_uc_fw_fini(&guc->fw);
 
 	guc_free_load_err_log(guc);
 }
@@ -252,6 +239,7 @@  static void guc_disable_communication(struct intel_guc *guc)
 int intel_uc_init_misc(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
+	struct intel_huc *huc = &i915->huc;
 	int ret;
 
 	if (!USES_GUC(i915))
@@ -261,16 +249,30 @@  int intel_uc_init_misc(struct drm_i915_private *i915)
 	if (ret)
 		return ret;
 
+	if (USES_HUC(i915)) {
+		ret = intel_huc_init_misc(huc);
+		if (ret)
+			goto err_guc;
+	}
+
 	return 0;
+
+err_guc:
+	intel_guc_fini_misc(guc);
+	return ret;
 }
 
 void intel_uc_fini_misc(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
+	struct intel_huc *huc = &i915->huc;
 
 	if (!USES_GUC(i915))
 		return;
 
+	if (USES_HUC(i915))
+		intel_huc_fini_misc(huc);
+
 	intel_guc_fini_misc(guc);
 }