All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init
@ 2018-06-15 19:20 Michal Wajdeczko
  2018-06-15 20:00 ` Michel Thierry
  2018-06-15 21:49 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 3+ messages in thread
From: Michal Wajdeczko @ 2018-06-15 19:20 UTC (permalink / raw)
  To: intel-gfx

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. Move fw fetching to
init phase and do it separately per guc/huc struct.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c |  7 ++++++-
 drivers/gpu/drm/i915/intel_huc.c |  8 ++++++++
 drivers/gpu/drm/i915/intel_huc.h |  6 ++++++
 drivers/gpu/drm/i915/intel_uc.c  | 37 ++++++++++++++++++++-----------------
 4 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index f84fbde..b698f23 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -167,9 +167,11 @@ int intel_guc_init(struct intel_guc *guc)
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	int ret;
 
+	intel_uc_fw_fetch(dev_priv, &guc->fw);
+
 	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);
@@ -190,6 +192,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;
 }
 
@@ -201,6 +205,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..8a884d7 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(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..21e600c 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(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(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 94e8863..ec3c37c 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);
 }
@@ -279,6 +266,7 @@ void intel_uc_fini_misc(struct drm_i915_private *i915)
 int intel_uc_init(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
+	struct intel_huc *huc = &i915->huc;
 	int ret;
 
 	if (!USES_GUC(i915))
@@ -291,24 +279,36 @@ int intel_uc_init(struct drm_i915_private *i915)
 	if (ret)
 		return ret;
 
+	if (USES_HUC(i915)) {
+		ret = intel_huc_init(huc);
+		if (ret)
+			goto err_guc;
+	}
+
 	if (USES_GUC_SUBMISSION(i915)) {
 		/*
 		 * This is stuff we need to have available at fw load time
 		 * if we are planning to enable submission later
 		 */
 		ret = intel_guc_submission_init(guc);
-		if (ret) {
-			intel_guc_fini(guc);
-			return ret;
-		}
+		if (ret)
+			goto err_huc;
 	}
 
 	return 0;
+
+err_huc:
+	if (USES_HUC(i915))
+		intel_huc_fini(huc);
+err_guc:
+	intel_guc_fini(guc);
+	return ret;
 }
 
 void intel_uc_fini(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
+	struct intel_huc *huc = &i915->huc;
 
 	if (!USES_GUC(i915))
 		return;
@@ -318,6 +318,9 @@ void intel_uc_fini(struct drm_i915_private *i915)
 	if (USES_GUC_SUBMISSION(i915))
 		intel_guc_submission_fini(guc);
 
+	if (USES_HUC(i915))
+		intel_huc_fini(huc);
+
 	intel_guc_fini(guc);
 }
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init
  2018-06-15 19:20 [PATCH] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init Michal Wajdeczko
@ 2018-06-15 20:00 ` Michel Thierry
  2018-06-15 21:49 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Michel Thierry @ 2018-06-15 20:00 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx

Hi,


On 6/15/2018 12:20 PM, 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. Move fw fetching to
> init phase and do it separately per guc/huc struct.
>

Can you resend this as a 2-patch series and with the "HAX enable guc, 
huc for CI" as the second patch? We want some CI testing for this.

Also, won't this should also move intel_wopcm_init()?
We need the guc/huc fw size for the wopcm stuff.

> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c |  7 ++++++-
>   drivers/gpu/drm/i915/intel_huc.c |  8 ++++++++
>   drivers/gpu/drm/i915/intel_huc.h |  6 ++++++
>   drivers/gpu/drm/i915/intel_uc.c  | 37 ++++++++++++++++++++-----------------
>   4 files changed, 40 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index f84fbde..b698f23 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -167,9 +167,11 @@ int intel_guc_init(struct intel_guc *guc)
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   	int ret;
>   
> +	intel_uc_fw_fetch(dev_priv, &guc->fw);
> +
>   	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);
> @@ -190,6 +192,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;
>   }
>   
> @@ -201,6 +205,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..8a884d7 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(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..21e600c 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(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(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 94e8863..ec3c37c 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);
>   }
> @@ -279,6 +266,7 @@ void intel_uc_fini_misc(struct drm_i915_private *i915)
>   int intel_uc_init(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> +	struct intel_huc *huc = &i915->huc;
>   	int ret;
>   
>   	if (!USES_GUC(i915))
> @@ -291,24 +279,36 @@ int intel_uc_init(struct drm_i915_private *i915)
>   	if (ret)
>   		return ret;
>   
> +	if (USES_HUC(i915)) {
> +		ret = intel_huc_init(huc);
> +		if (ret)
> +			goto err_guc;
> +	}
> +
>   	if (USES_GUC_SUBMISSION(i915)) {
>   		/*
>   		 * This is stuff we need to have available at fw load time
>   		 * if we are planning to enable submission later
>   		 */
>   		ret = intel_guc_submission_init(guc);
> -		if (ret) {
> -			intel_guc_fini(guc);
> -			return ret;
> -		}
> +		if (ret)
> +			goto err_huc;
>   	}
>   
>   	return 0;
> +
> +err_huc:
> +	if (USES_HUC(i915))
> +		intel_huc_fini(huc);
> +err_guc:
> +	intel_guc_fini(guc);
> +	return ret;
>   }
>   
>   void intel_uc_fini(struct drm_i915_private *i915)
>   {
>   	struct intel_guc *guc = &i915->guc;
> +	struct intel_huc *huc = &i915->huc;
>   
>   	if (!USES_GUC(i915))
>   		return;
> @@ -318,6 +318,9 @@ void intel_uc_fini(struct drm_i915_private *i915)
>   	if (USES_GUC_SUBMISSION(i915))
>   		intel_guc_submission_fini(guc);
>   
> +	if (USES_HUC(i915))
> +		intel_huc_fini(huc);
> +
>   	intel_guc_fini(guc);
>   }
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init
  2018-06-15 19:20 [PATCH] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init Michal Wajdeczko
  2018-06-15 20:00 ` Michel Thierry
@ 2018-06-15 21:49 ` Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2018-06-15 21:49 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init
URL   : https://patchwork.freedesktop.org/series/44861/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4328 -> Patchwork_9338 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9338 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9338, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44861/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9338:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload:
      fi-skl-guc:         PASS -> DMESG-FAIL +2

    igt@drv_module_reload@basic-reload-inject:
      fi-kbl-guc:         PASS -> DMESG-FAIL +2

    
    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    igt@gem_mmap_gtt@basic-wc:
      fi-kbl-guc:         PASS -> SKIP +206

    igt@prime_self_import@basic-llseek-bad:
      fi-skl-guc:         PASS -> SKIP +243

    
== Known issues ==

  Here are the changes found in Patchwork_9338 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000) +2

    igt@kms_flip@basic-plain-flip:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106238)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-glk-j4005:       DMESG-WARN (fdo#106238) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106238 https://bugs.freedesktop.org/show_bug.cgi?id=106238


== Participating hosts (43 -> 37) ==

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4328 -> Patchwork_9338

  CI_DRM_4328: 6d317aed03baa49c412638f69ffe580001c7d3b6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4521: 4aa49a88acdaafed8235837d85a099ad941fc281 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9338: 495c005c0be22c736d28ebf7b4d204fd6f3eef29 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

495c005c0be2 drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9338/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-06-15 21:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-15 19:20 [PATCH] drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc specific init Michal Wajdeczko
2018-06-15 20:00 ` Michel Thierry
2018-06-15 21:49 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.