* [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure
@ 2018-03-13 13:54 Michal Wajdeczko
2018-03-13 13:54 ` [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Michal Wajdeczko @ 2018-03-13 13:54 UTC (permalink / raw)
To: intel-gfx
Since commit 6ca9a2beb54a ("drm/i915: Unwind i915_gem_init() failure")
we believed that we correctly handle all errors encountered during
GuC initialization, including special one that indicates request to
run driver with disabled GPU submission (-EIO).
Unfortunately since commit 121981fafe69 ("drm/i915/guc: Combine
enable_guc_loading|submission modparams") we stopped using that
error code to avoid unwanted fallback to execlist submission mode.
In result any GuC initialization failure was treated as non-recoverable
error leading to driver load abort, so we could not even read related
GuC error log to investigate cause of the problem.
Fix that by always returning -EIO on uC hardware related failure.
v2: don't allow -EIO from uc_init
don't call uc_fini[_misc] on -EIO
mark guc fw as failed on hw init failure
prepare uc_fini_hw to run after earlier -EIO
v3: update comments (Sagar)
use sanitize functions on failure in init_hw (Michal)
and also sanitize guc/huc fw in fini_hw (Michal)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++-------
drivers/gpu/drm/i915/intel_guc.h | 5 +++++
drivers/gpu/drm/i915/intel_uc.c | 18 ++++++++++++++----
drivers/gpu/drm/i915/intel_uc_fw.h | 5 +++++
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 05b0724..8eed87d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5338,8 +5338,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
intel_init_gt_powersave(dev_priv);
ret = intel_uc_init(dev_priv);
- if (ret)
+ if (ret) {
+ GEM_BUG_ON(ret == -EIO);
goto err_pm;
+ }
ret = i915_gem_init_hw(dev_priv);
if (ret)
@@ -5386,7 +5388,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
i915_gem_contexts_lost(dev_priv);
intel_uc_fini_hw(dev_priv);
err_uc_init:
- intel_uc_fini(dev_priv);
+ if (ret != -EIO)
+ intel_uc_fini(dev_priv);
err_pm:
if (ret != -EIO) {
intel_cleanup_gt_powersave(dev_priv);
@@ -5400,15 +5403,15 @@ 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);
- intel_uc_fini_misc(dev_priv);
-
- if (ret != -EIO)
+ if (ret != -EIO) {
+ intel_uc_fini_misc(dev_priv);
i915_gem_cleanup_userptr(dev_priv);
+ }
if (ret == -EIO) {
/*
- * Allow engine initialisation to fail by marking the GPU as
- * wedged. But we only want to do this where the GPU is angry,
+ * Allow engines or uC initialization to fail by marking the GPU
+ * as wedged. But we only want to do this when the GPU is angry,
* for all other failure, such as an allocation failure, bail.
*/
if (!i915_terminally_wedged(&dev_priv->gpu_error)) {
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index d878160..faa9e01 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -139,4 +139,9 @@ static inline int intel_guc_sanitize(struct intel_guc *guc)
return 0;
}
+static inline bool intel_guc_is_loaded(struct intel_guc *guc)
+{
+ return intel_uc_fw_is_loaded(&guc->fw);
+}
+
#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9d5ffd7..f89acf4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -447,15 +447,20 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
* Note that there is no fallback as either user explicitly asked for
* the GuC or driver default option was to run with the GuC enabled.
*/
- if (GEM_WARN_ON(ret == -EIO))
- ret = -EINVAL;
-
dev_err(dev_priv->drm.dev, "GuC initialization failed %d\n", ret);
- return ret;
+
+ /* Sanitize GuC/HuC to avoid clean-up on wedged */
+ intel_huc_sanitize(huc);
+ intel_guc_sanitize(guc);
+ GEM_BUG_ON(intel_guc_is_loaded(guc));
+
+ /* We want to disable GPU submission but keep KMS alive */
+ return -EIO;
}
void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
{
+ struct intel_huc *huc = &dev_priv->huc;
struct intel_guc *guc = &dev_priv->guc;
if (!USES_GUC(dev_priv))
@@ -463,10 +468,15 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
GEM_BUG_ON(!HAS_GUC(dev_priv));
+ if (!intel_guc_is_loaded(guc))
+ return;
+
if (USES_GUC_SUBMISSION(dev_priv))
intel_guc_submission_disable(guc);
guc_disable_communication(guc);
+ intel_huc_sanitize(huc);
+ intel_guc_sanitize(guc);
if (USES_GUC_SUBMISSION(dev_priv))
gen9_disable_guc_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index 2601521..ff9ce9a 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -121,6 +121,11 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
}
+static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
+{
+ return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+}
+
void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
struct intel_uc_fw *uc_fw);
int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
--
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] 9+ messages in thread
* [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status
2018-03-13 13:54 [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
@ 2018-03-13 13:54 ` Michal Wajdeczko
2018-03-14 6:49 ` Sagar Arun Kamble
2018-03-13 13:54 ` [PATCH v3 3/4] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Michal Wajdeczko @ 2018-03-13 13:54 UTC (permalink / raw)
To: intel-gfx
We don't have to check load status values.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_huc.c | 2 +-
drivers/gpu/drm/i915/intel_uc.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 65e2afb..5c3423a 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -51,7 +51,7 @@ int intel_huc_auth(struct intel_huc *huc)
u32 status;
int ret;
- if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ if (!intel_uc_fw_is_loaded(&huc->fw))
return -ENOEXEC;
vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f89acf4..0bc8f3b 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -490,7 +490,7 @@ int intel_uc_suspend(struct drm_i915_private *i915)
if (!USES_GUC(i915))
return 0;
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ if (!intel_guc_is_loaded(guc))
return 0;
err = intel_guc_suspend(guc);
@@ -512,7 +512,7 @@ int intel_uc_resume(struct drm_i915_private *i915)
if (!USES_GUC(i915))
return 0;
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ if (!intel_guc_is_loaded(guc))
return 0;
if (i915_modparams.guc_log_level)
--
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] 9+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status
2018-03-13 13:54 ` [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
@ 2018-03-14 6:49 ` Sagar Arun Kamble
0 siblings, 0 replies; 9+ messages in thread
From: Sagar Arun Kamble @ 2018-03-14 6:49 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 3/13/2018 7:24 PM, Michal Wajdeczko wrote:
> We don't have to check load status values.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/intel_huc.c | 2 +-
> drivers/gpu/drm/i915/intel_uc.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 65e2afb..5c3423a 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -51,7 +51,7 @@ int intel_huc_auth(struct intel_huc *huc)
> u32 status;
> int ret;
>
> - if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> + if (!intel_uc_fw_is_loaded(&huc->fw))
> return -ENOEXEC;
>
> vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index f89acf4..0bc8f3b 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -490,7 +490,7 @@ int intel_uc_suspend(struct drm_i915_private *i915)
> if (!USES_GUC(i915))
> return 0;
>
> - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> + if (!intel_guc_is_loaded(guc))
> return 0;
>
> err = intel_guc_suspend(guc);
> @@ -512,7 +512,7 @@ int intel_uc_resume(struct drm_i915_private *i915)
> if (!USES_GUC(i915))
> return 0;
>
> - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> + if (!intel_guc_is_loaded(guc))
> return 0;
>
> if (i915_modparams.guc_log_level)
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/4] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
2018-03-13 13:54 [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
2018-03-13 13:54 ` [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
@ 2018-03-13 13:54 ` Michal Wajdeczko
2018-03-14 6:57 ` Sagar Arun Kamble
2018-03-13 13:54 ` [PATCH v3 4/4] HAX: Enable GuC for CI Michal Wajdeczko
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Michal Wajdeczko @ 2018-03-13 13:54 UTC (permalink / raw)
To: intel-gfx
Some functions already use i915 name instead of dev_priv.
Let's rename this param in all remaining functions, except
those that still use legacy macros.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/intel_uc.c | 122 ++++++++++++++++++++--------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0bc8f3b..c22155b 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -50,10 +50,10 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
return ret;
}
-static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
+static int __get_platform_enable_guc(struct drm_i915_private *i915)
{
- struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
- struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+ struct intel_uc_fw *guc_fw = &i915->guc.fw;
+ struct intel_uc_fw *huc_fw = &i915->huc.fw;
int enable_guc = 0;
/* Default is to enable GuC/HuC if we know their firmwares */
@@ -67,12 +67,12 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
return enable_guc;
}
-static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
+static int __get_default_guc_log_level(struct drm_i915_private *i915)
{
int guc_log_level = 0; /* disabled */
/* Enable if we're running on platform with GuC and debug config */
- if (HAS_GUC(dev_priv) && intel_uc_is_using_guc() &&
+ if (HAS_GUC(i915) && intel_uc_is_using_guc() &&
(IS_ENABLED(CONFIG_DRM_I915_DEBUG) ||
IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)))
guc_log_level = 1 + GUC_LOG_VERBOSITY_MAX;
@@ -99,14 +99,14 @@ static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
* unless GuC is enabled on given platform and the driver is compiled with
* debug config when this modparam will default to "enable(1..4)".
*/
-static void sanitize_options_early(struct drm_i915_private *dev_priv)
+static void sanitize_options_early(struct drm_i915_private *i915)
{
- struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
- struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+ struct intel_uc_fw *guc_fw = &i915->guc.fw;
+ struct intel_uc_fw *huc_fw = &i915->huc.fw;
/* A negative value means "use platform default" */
if (i915_modparams.enable_guc < 0)
- i915_modparams.enable_guc = __get_platform_enable_guc(dev_priv);
+ i915_modparams.enable_guc = __get_platform_enable_guc(i915);
DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
i915_modparams.enable_guc,
@@ -117,28 +117,28 @@ static void sanitize_options_early(struct drm_i915_private *dev_priv)
if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
"enable_guc", i915_modparams.enable_guc,
- !HAS_GUC(dev_priv) ? "no GuC hardware" :
- "no GuC firmware");
+ !HAS_GUC(i915) ? "no GuC hardware" :
+ "no GuC firmware");
}
/* Verify HuC firmware availability */
if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
"enable_guc", i915_modparams.enable_guc,
- !HAS_HUC(dev_priv) ? "no HuC hardware" :
- "no HuC firmware");
+ !HAS_HUC(i915) ? "no HuC hardware" :
+ "no HuC firmware");
}
/* A negative value means "use platform/config default" */
if (i915_modparams.guc_log_level < 0)
i915_modparams.guc_log_level =
- __get_default_guc_log_level(dev_priv);
+ __get_default_guc_log_level(i915);
if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc()) {
DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
"guc_log_level", i915_modparams.guc_log_level,
- !HAS_GUC(dev_priv) ? "no GuC hardware" :
- "GuC not enabled");
+ !HAS_GUC(i915) ? "no GuC hardware" :
+ "GuC not enabled");
i915_modparams.guc_log_level = 0;
}
@@ -159,36 +159,36 @@ static void sanitize_options_early(struct drm_i915_private *dev_priv)
GEM_BUG_ON(i915_modparams.guc_log_level < 0);
}
-void intel_uc_init_early(struct drm_i915_private *dev_priv)
+void intel_uc_init_early(struct drm_i915_private *i915)
{
- intel_guc_init_early(&dev_priv->guc);
- intel_huc_init_early(&dev_priv->huc);
+ intel_guc_init_early(&i915->guc);
+ intel_huc_init_early(&i915->huc);
- sanitize_options_early(dev_priv);
+ sanitize_options_early(i915);
}
-void intel_uc_init_fw(struct drm_i915_private *dev_priv)
+void intel_uc_init_fw(struct drm_i915_private *i915)
{
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return;
- if (USES_HUC(dev_priv))
- intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
+ if (USES_HUC(i915))
+ intel_uc_fw_fetch(i915, &i915->huc.fw);
- intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
+ intel_uc_fw_fetch(i915, &i915->guc.fw);
}
-void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
+void intel_uc_fini_fw(struct drm_i915_private *i915)
{
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return;
- intel_uc_fw_fini(&dev_priv->guc.fw);
+ intel_uc_fw_fini(&i915->guc.fw);
- if (USES_HUC(dev_priv))
- intel_uc_fw_fini(&dev_priv->huc.fw);
+ if (USES_HUC(i915))
+ intel_uc_fw_fini(&i915->huc.fw);
- guc_free_load_err_log(&dev_priv->guc);
+ guc_free_load_err_log(&i915->guc);
}
/**
@@ -199,9 +199,9 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
* Setup minimal state necessary for MMIO accesses later in the
* initialization sequence.
*/
-void intel_uc_init_mmio(struct drm_i915_private *dev_priv)
+void intel_uc_init_mmio(struct drm_i915_private *i915)
{
- intel_guc_init_send_regs(&dev_priv->guc);
+ intel_guc_init_send_regs(&i915->guc);
}
static void guc_capture_load_err_log(struct intel_guc *guc)
@@ -245,9 +245,9 @@ void intel_uc_unregister(struct drm_i915_private *i915)
static int guc_enable_communication(struct intel_guc *guc)
{
- struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ struct drm_i915_private *i915 = guc_to_i915(guc);
- if (HAS_GUC_CT(dev_priv))
+ if (HAS_GUC_CT(i915))
return intel_guc_enable_ct(guc);
guc->send = intel_guc_send_mmio;
@@ -256,20 +256,20 @@ static int guc_enable_communication(struct intel_guc *guc)
static void guc_disable_communication(struct intel_guc *guc)
{
- struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ struct drm_i915_private *i915 = guc_to_i915(guc);
- if (HAS_GUC_CT(dev_priv))
+ if (HAS_GUC_CT(i915))
intel_guc_disable_ct(guc);
guc->send = intel_guc_send_nop;
}
-int intel_uc_init_misc(struct drm_i915_private *dev_priv)
+int intel_uc_init_misc(struct drm_i915_private *i915)
{
- struct intel_guc *guc = &dev_priv->guc;
+ struct intel_guc *guc = &i915->guc;
int ret;
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return 0;
ret = intel_guc_init_wq(guc);
@@ -279,32 +279,32 @@ int intel_uc_init_misc(struct drm_i915_private *dev_priv)
return 0;
}
-void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
+void intel_uc_fini_misc(struct drm_i915_private *i915)
{
- struct intel_guc *guc = &dev_priv->guc;
+ struct intel_guc *guc = &i915->guc;
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return;
intel_guc_fini_wq(guc);
}
-int intel_uc_init(struct drm_i915_private *dev_priv)
+int intel_uc_init(struct drm_i915_private *i915)
{
- struct intel_guc *guc = &dev_priv->guc;
+ struct intel_guc *guc = &i915->guc;
int ret;
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return 0;
- if (!HAS_GUC(dev_priv))
+ if (!HAS_GUC(i915))
return -ENODEV;
ret = intel_guc_init(guc);
if (ret)
return ret;
- if (USES_GUC_SUBMISSION(dev_priv)) {
+ 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
@@ -319,16 +319,16 @@ int intel_uc_init(struct drm_i915_private *dev_priv)
return 0;
}
-void intel_uc_fini(struct drm_i915_private *dev_priv)
+void intel_uc_fini(struct drm_i915_private *i915)
{
- struct intel_guc *guc = &dev_priv->guc;
+ struct intel_guc *guc = &i915->guc;
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return;
- GEM_BUG_ON(!HAS_GUC(dev_priv));
+ GEM_BUG_ON(!HAS_GUC(i915));
- if (USES_GUC_SUBMISSION(dev_priv))
+ if (USES_GUC_SUBMISSION(i915))
intel_guc_submission_fini(guc);
intel_guc_fini(guc);
@@ -458,28 +458,28 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
return -EIO;
}
-void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
+void intel_uc_fini_hw(struct drm_i915_private *i915)
{
- struct intel_huc *huc = &dev_priv->huc;
- struct intel_guc *guc = &dev_priv->guc;
+ struct intel_huc *huc = &i915->huc;
+ struct intel_guc *guc = &i915->guc;
- if (!USES_GUC(dev_priv))
+ if (!USES_GUC(i915))
return;
- GEM_BUG_ON(!HAS_GUC(dev_priv));
+ GEM_BUG_ON(!HAS_GUC(i915));
if (!intel_guc_is_loaded(guc))
return;
- if (USES_GUC_SUBMISSION(dev_priv))
+ if (USES_GUC_SUBMISSION(i915))
intel_guc_submission_disable(guc);
guc_disable_communication(guc);
intel_huc_sanitize(huc);
intel_guc_sanitize(guc);
- if (USES_GUC_SUBMISSION(dev_priv))
- gen9_disable_guc_interrupts(dev_priv);
+ if (USES_GUC_SUBMISSION(i915))
+ gen9_disable_guc_interrupts(i915);
}
int intel_uc_suspend(struct drm_i915_private *i915)
--
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] 9+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
2018-03-13 13:54 ` [PATCH v3 3/4] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
@ 2018-03-14 6:57 ` Sagar Arun Kamble
0 siblings, 0 replies; 9+ messages in thread
From: Sagar Arun Kamble @ 2018-03-14 6:57 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 3/13/2018 7:24 PM, Michal Wajdeczko wrote:
> Some functions already use i915 name instead of dev_priv.
> Let's rename this param in all remaining functions, except
> those that still use legacy macros.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Function comment description is not updated for sanitize_options_early
and intel_uc_init_mmio.
With that:
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 122 ++++++++++++++++++++--------------------
> 1 file changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 0bc8f3b..c22155b 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -50,10 +50,10 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
> return ret;
> }
>
> -static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
> +static int __get_platform_enable_guc(struct drm_i915_private *i915)
> {
> - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> - struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
> + struct intel_uc_fw *guc_fw = &i915->guc.fw;
> + struct intel_uc_fw *huc_fw = &i915->huc.fw;
> int enable_guc = 0;
>
> /* Default is to enable GuC/HuC if we know their firmwares */
> @@ -67,12 +67,12 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
> return enable_guc;
> }
>
> -static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
> +static int __get_default_guc_log_level(struct drm_i915_private *i915)
> {
> int guc_log_level = 0; /* disabled */
>
> /* Enable if we're running on platform with GuC and debug config */
> - if (HAS_GUC(dev_priv) && intel_uc_is_using_guc() &&
> + if (HAS_GUC(i915) && intel_uc_is_using_guc() &&
> (IS_ENABLED(CONFIG_DRM_I915_DEBUG) ||
> IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)))
> guc_log_level = 1 + GUC_LOG_VERBOSITY_MAX;
> @@ -99,14 +99,14 @@ static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
> * unless GuC is enabled on given platform and the driver is compiled with
> * debug config when this modparam will default to "enable(1..4)".
> */
> -static void sanitize_options_early(struct drm_i915_private *dev_priv)
> +static void sanitize_options_early(struct drm_i915_private *i915)
> {
> - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> - struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
> + struct intel_uc_fw *guc_fw = &i915->guc.fw;
> + struct intel_uc_fw *huc_fw = &i915->huc.fw;
>
> /* A negative value means "use platform default" */
> if (i915_modparams.enable_guc < 0)
> - i915_modparams.enable_guc = __get_platform_enable_guc(dev_priv);
> + i915_modparams.enable_guc = __get_platform_enable_guc(i915);
>
> DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
> i915_modparams.enable_guc,
> @@ -117,28 +117,28 @@ static void sanitize_options_early(struct drm_i915_private *dev_priv)
> if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
> DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> "enable_guc", i915_modparams.enable_guc,
> - !HAS_GUC(dev_priv) ? "no GuC hardware" :
> - "no GuC firmware");
> + !HAS_GUC(i915) ? "no GuC hardware" :
> + "no GuC firmware");
> }
>
> /* Verify HuC firmware availability */
> if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
> DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> "enable_guc", i915_modparams.enable_guc,
> - !HAS_HUC(dev_priv) ? "no HuC hardware" :
> - "no HuC firmware");
> + !HAS_HUC(i915) ? "no HuC hardware" :
> + "no HuC firmware");
> }
>
> /* A negative value means "use platform/config default" */
> if (i915_modparams.guc_log_level < 0)
> i915_modparams.guc_log_level =
> - __get_default_guc_log_level(dev_priv);
> + __get_default_guc_log_level(i915);
>
> if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc()) {
> DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> "guc_log_level", i915_modparams.guc_log_level,
> - !HAS_GUC(dev_priv) ? "no GuC hardware" :
> - "GuC not enabled");
> + !HAS_GUC(i915) ? "no GuC hardware" :
> + "GuC not enabled");
> i915_modparams.guc_log_level = 0;
> }
>
> @@ -159,36 +159,36 @@ static void sanitize_options_early(struct drm_i915_private *dev_priv)
> GEM_BUG_ON(i915_modparams.guc_log_level < 0);
> }
>
> -void intel_uc_init_early(struct drm_i915_private *dev_priv)
> +void intel_uc_init_early(struct drm_i915_private *i915)
> {
> - intel_guc_init_early(&dev_priv->guc);
> - intel_huc_init_early(&dev_priv->huc);
> + intel_guc_init_early(&i915->guc);
> + intel_huc_init_early(&i915->huc);
>
> - sanitize_options_early(dev_priv);
> + sanitize_options_early(i915);
> }
>
> -void intel_uc_init_fw(struct drm_i915_private *dev_priv)
> +void intel_uc_init_fw(struct drm_i915_private *i915)
> {
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return;
>
> - if (USES_HUC(dev_priv))
> - intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
> + if (USES_HUC(i915))
> + intel_uc_fw_fetch(i915, &i915->huc.fw);
>
> - intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw);
> + intel_uc_fw_fetch(i915, &i915->guc.fw);
> }
>
> -void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> +void intel_uc_fini_fw(struct drm_i915_private *i915)
> {
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return;
>
> - intel_uc_fw_fini(&dev_priv->guc.fw);
> + intel_uc_fw_fini(&i915->guc.fw);
>
> - if (USES_HUC(dev_priv))
> - intel_uc_fw_fini(&dev_priv->huc.fw);
> + if (USES_HUC(i915))
> + intel_uc_fw_fini(&i915->huc.fw);
>
> - guc_free_load_err_log(&dev_priv->guc);
> + guc_free_load_err_log(&i915->guc);
> }
>
> /**
> @@ -199,9 +199,9 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> * Setup minimal state necessary for MMIO accesses later in the
> * initialization sequence.
> */
> -void intel_uc_init_mmio(struct drm_i915_private *dev_priv)
> +void intel_uc_init_mmio(struct drm_i915_private *i915)
> {
> - intel_guc_init_send_regs(&dev_priv->guc);
> + intel_guc_init_send_regs(&i915->guc);
> }
>
> static void guc_capture_load_err_log(struct intel_guc *guc)
> @@ -245,9 +245,9 @@ void intel_uc_unregister(struct drm_i915_private *i915)
>
> static int guc_enable_communication(struct intel_guc *guc)
> {
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + struct drm_i915_private *i915 = guc_to_i915(guc);
>
> - if (HAS_GUC_CT(dev_priv))
> + if (HAS_GUC_CT(i915))
> return intel_guc_enable_ct(guc);
>
> guc->send = intel_guc_send_mmio;
> @@ -256,20 +256,20 @@ static int guc_enable_communication(struct intel_guc *guc)
>
> static void guc_disable_communication(struct intel_guc *guc)
> {
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + struct drm_i915_private *i915 = guc_to_i915(guc);
>
> - if (HAS_GUC_CT(dev_priv))
> + if (HAS_GUC_CT(i915))
> intel_guc_disable_ct(guc);
>
> guc->send = intel_guc_send_nop;
> }
>
> -int intel_uc_init_misc(struct drm_i915_private *dev_priv)
> +int intel_uc_init_misc(struct drm_i915_private *i915)
> {
> - struct intel_guc *guc = &dev_priv->guc;
> + struct intel_guc *guc = &i915->guc;
> int ret;
>
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return 0;
>
> ret = intel_guc_init_wq(guc);
> @@ -279,32 +279,32 @@ int intel_uc_init_misc(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> -void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
> +void intel_uc_fini_misc(struct drm_i915_private *i915)
> {
> - struct intel_guc *guc = &dev_priv->guc;
> + struct intel_guc *guc = &i915->guc;
>
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return;
>
> intel_guc_fini_wq(guc);
> }
>
> -int intel_uc_init(struct drm_i915_private *dev_priv)
> +int intel_uc_init(struct drm_i915_private *i915)
> {
> - struct intel_guc *guc = &dev_priv->guc;
> + struct intel_guc *guc = &i915->guc;
> int ret;
>
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return 0;
>
> - if (!HAS_GUC(dev_priv))
> + if (!HAS_GUC(i915))
> return -ENODEV;
>
> ret = intel_guc_init(guc);
> if (ret)
> return ret;
>
> - if (USES_GUC_SUBMISSION(dev_priv)) {
> + 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
> @@ -319,16 +319,16 @@ int intel_uc_init(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> -void intel_uc_fini(struct drm_i915_private *dev_priv)
> +void intel_uc_fini(struct drm_i915_private *i915)
> {
> - struct intel_guc *guc = &dev_priv->guc;
> + struct intel_guc *guc = &i915->guc;
>
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return;
>
> - GEM_BUG_ON(!HAS_GUC(dev_priv));
> + GEM_BUG_ON(!HAS_GUC(i915));
>
> - if (USES_GUC_SUBMISSION(dev_priv))
> + if (USES_GUC_SUBMISSION(i915))
> intel_guc_submission_fini(guc);
>
> intel_guc_fini(guc);
> @@ -458,28 +458,28 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> return -EIO;
> }
>
> -void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> +void intel_uc_fini_hw(struct drm_i915_private *i915)
> {
> - struct intel_huc *huc = &dev_priv->huc;
> - struct intel_guc *guc = &dev_priv->guc;
> + struct intel_huc *huc = &i915->huc;
> + struct intel_guc *guc = &i915->guc;
>
> - if (!USES_GUC(dev_priv))
> + if (!USES_GUC(i915))
> return;
>
> - GEM_BUG_ON(!HAS_GUC(dev_priv));
> + GEM_BUG_ON(!HAS_GUC(i915));
>
> if (!intel_guc_is_loaded(guc))
> return;
>
> - if (USES_GUC_SUBMISSION(dev_priv))
> + if (USES_GUC_SUBMISSION(i915))
> intel_guc_submission_disable(guc);
>
> guc_disable_communication(guc);
> intel_huc_sanitize(huc);
> intel_guc_sanitize(guc);
>
> - if (USES_GUC_SUBMISSION(dev_priv))
> - gen9_disable_guc_interrupts(dev_priv);
> + if (USES_GUC_SUBMISSION(i915))
> + gen9_disable_guc_interrupts(i915);
> }
>
> int intel_uc_suspend(struct drm_i915_private *i915)
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 4/4] HAX: Enable GuC for CI
2018-03-13 13:54 [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
2018-03-13 13:54 ` [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
2018-03-13 13:54 ` [PATCH v3 3/4] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
@ 2018-03-13 13:54 ` Michal Wajdeczko
2018-03-13 15:45 ` ✗ Fi.CI.BAT: warning for series starting with [v3,1/4] drm/i915/uc: Use correct error code for GuC initialization failure Patchwork
2018-03-14 6:42 ` [PATCH v3 1/4] " Sagar Arun Kamble
4 siblings, 0 replies; 9+ messages in thread
From: Michal Wajdeczko @ 2018-03-13 13:54 UTC (permalink / raw)
To: intel-gfx
v2: except running with HYPERVISOR
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
drivers/gpu/drm/i915/intel_uc.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 430f5f9..3deae1e 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc, 0) \
+ param(int, enable_guc, -1) \
param(int, guc_log_level, 0) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c22155b..22af945 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *i915)
enable_guc |= ENABLE_GUC_LOAD_HUC;
/* Any platform specific fine-tuning can be done here */
+ if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+ enable_guc = 0;
return enable_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] 9+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [v3,1/4] drm/i915/uc: Use correct error code for GuC initialization failure
2018-03-13 13:54 [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
` (2 preceding siblings ...)
2018-03-13 13:54 ` [PATCH v3 4/4] HAX: Enable GuC for CI Michal Wajdeczko
@ 2018-03-13 15:45 ` Patchwork
2018-03-14 6:42 ` [PATCH v3 1/4] " Sagar Arun Kamble
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-13 15:45 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915/uc: Use correct error code for GuC initialization failure
URL : https://patchwork.freedesktop.org/series/39867/
State : warning
== Summary ==
Series 39867v1 series starting with [v3,1/4] drm/i915/uc: Use correct error code for GuC initialization failure
https://patchwork.freedesktop.org/api/1.0/series/39867/revisions/1/mbox/
---- Possible new issues:
Test drv_module_reload:
Subgroup basic-no-display:
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-cfl-8700k)
pass -> DMESG-WARN (fi-cfl-s2)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-r)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-guc)
Subgroup basic-reload:
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-cfl-8700k)
pass -> DMESG-WARN (fi-cfl-s2)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-r)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-guc)
Subgroup basic-reload-inject:
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-cfl-8700k)
pass -> DMESG-WARN (fi-cfl-s2)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-WARN (fi-kbl-r)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k2)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-guc)
Test kms_busy:
Subgroup basic-flip-a:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-b:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-c:
skip -> PASS (fi-ivb-3770)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
skip -> PASS (fi-ivb-3770)
Subgroup basic-busy-flip-before-cursor-legacy:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-after-cursor-atomic:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-after-cursor-legacy:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-after-cursor-varying-size:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-before-cursor-atomic:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-before-cursor-legacy:
skip -> PASS (fi-ivb-3770)
Subgroup basic-flip-before-cursor-varying-size:
skip -> PASS (fi-ivb-3770)
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
pass -> SKIP (fi-ivb-3770)
Test kms_frontbuffer_tracking:
Subgroup basic:
skip -> PASS (fi-ivb-3770)
---- Known issues:
Test drv_module_reload:
Subgroup basic-reload:
pass -> DMESG-WARN (fi-kbl-7567u) fdo#105078 +2
Test kms_chamelium:
Subgroup dp-crc-fast:
fail -> PASS (fi-kbl-7500u) fdo#103841
Test prime_vgem:
Subgroup basic-fence-flip:
skip -> PASS (fi-ivb-3770) fdo#104008
fdo#105078 https://bugs.freedesktop.org/show_bug.cgi?id=105078
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:428s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:438s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:382s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:532s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:298s
fi-bxt-j4205 total:288 pass:256 dwarn:3 dfail:0 fail:0 skip:29 time:511s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:511s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:495s
fi-cfl-8700k total:288 pass:257 dwarn:3 dfail:0 fail:0 skip:28 time:417s
fi-cfl-s2 total:288 pass:259 dwarn:3 dfail:0 fail:0 skip:26 time:586s
fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:594s
WARNING: Long output truncated
874b86a759851707e26286c22062f6ccc526e46f drm-tip: 2018y-03m-13d-12h-36m-17s UTC integration manifest
8dbd1dc6b088 HAX: Enable GuC for CI
9a7d81da9200 drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
0be7883e23c6 drm/i915/uc: Use helper functions to detect fw load status
90dee3f8cd4a drm/i915/uc: Use correct error code for GuC initialization failure
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8328/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure
2018-03-13 13:54 [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
` (3 preceding siblings ...)
2018-03-13 15:45 ` ✗ Fi.CI.BAT: warning for series starting with [v3,1/4] drm/i915/uc: Use correct error code for GuC initialization failure Patchwork
@ 2018-03-14 6:42 ` Sagar Arun Kamble
4 siblings, 0 replies; 9+ messages in thread
From: Sagar Arun Kamble @ 2018-03-14 6:42 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 3/13/2018 7:24 PM, Michal Wajdeczko wrote:
> Since commit 6ca9a2beb54a ("drm/i915: Unwind i915_gem_init() failure")
> we believed that we correctly handle all errors encountered during
> GuC initialization, including special one that indicates request to
> run driver with disabled GPU submission (-EIO).
>
> Unfortunately since commit 121981fafe69 ("drm/i915/guc: Combine
> enable_guc_loading|submission modparams") we stopped using that
> error code to avoid unwanted fallback to execlist submission mode.
>
> In result any GuC initialization failure was treated as non-recoverable
> error leading to driver load abort, so we could not even read related
> GuC error log to investigate cause of the problem.
>
> Fix that by always returning -EIO on uC hardware related failure.
>
> v2: don't allow -EIO from uc_init
> don't call uc_fini[_misc] on -EIO
> mark guc fw as failed on hw init failure
> prepare uc_fini_hw to run after earlier -EIO
>
> v3: update comments (Sagar)
> use sanitize functions on failure in init_hw (Michal)
> and also sanitize guc/huc fw in fini_hw (Michal)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++-------
> drivers/gpu/drm/i915/intel_guc.h | 5 +++++
> drivers/gpu/drm/i915/intel_uc.c | 18 ++++++++++++++----
> drivers/gpu/drm/i915/intel_uc_fw.h | 5 +++++
> 4 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 05b0724..8eed87d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5338,8 +5338,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
> intel_init_gt_powersave(dev_priv);
>
> ret = intel_uc_init(dev_priv);
> - if (ret)
> + if (ret) {
> + GEM_BUG_ON(ret == -EIO);
> goto err_pm;
> + }
>
> ret = i915_gem_init_hw(dev_priv);
> if (ret)
> @@ -5386,7 +5388,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
> i915_gem_contexts_lost(dev_priv);
> intel_uc_fini_hw(dev_priv);
> err_uc_init:
> - intel_uc_fini(dev_priv);
> + if (ret != -EIO)
> + intel_uc_fini(dev_priv);
> err_pm:
> if (ret != -EIO) {
> intel_cleanup_gt_powersave(dev_priv);
> @@ -5400,15 +5403,15 @@ 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);
>
> - intel_uc_fini_misc(dev_priv);
> -
> - if (ret != -EIO)
> + if (ret != -EIO) {
> + intel_uc_fini_misc(dev_priv);
> i915_gem_cleanup_userptr(dev_priv);
> + }
>
> if (ret == -EIO) {
> /*
> - * Allow engine initialisation to fail by marking the GPU as
> - * wedged. But we only want to do this where the GPU is angry,
> + * Allow engines or uC initialization to fail by marking the GPU
> + * as wedged. But we only want to do this when the GPU is angry,
> * for all other failure, such as an allocation failure, bail.
> */
> if (!i915_terminally_wedged(&dev_priv->gpu_error)) {
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index d878160..faa9e01 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -139,4 +139,9 @@ static inline int intel_guc_sanitize(struct intel_guc *guc)
> return 0;
> }
>
> +static inline bool intel_guc_is_loaded(struct intel_guc *guc)
> +{
> + return intel_uc_fw_is_loaded(&guc->fw);
> +}
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 9d5ffd7..f89acf4 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -447,15 +447,20 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> * Note that there is no fallback as either user explicitly asked for
> * the GuC or driver default option was to run with the GuC enabled.
> */
> - if (GEM_WARN_ON(ret == -EIO))
> - ret = -EINVAL;
> -
> dev_err(dev_priv->drm.dev, "GuC initialization failed %d\n", ret);
> - return ret;
> +
> + /* Sanitize GuC/HuC to avoid clean-up on wedged */
> + intel_huc_sanitize(huc);
> + intel_guc_sanitize(guc);
> + GEM_BUG_ON(intel_guc_is_loaded(guc));
> +
How about also resetting the hw using __intel_uc_reset_hw().
> + /* We want to disable GPU submission but keep KMS alive */
> + return -EIO;
> }
>
> void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> {
> + struct intel_huc *huc = &dev_priv->huc;
> struct intel_guc *guc = &dev_priv->guc;
>
> if (!USES_GUC(dev_priv))
> @@ -463,10 +468,15 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
>
> GEM_BUG_ON(!HAS_GUC(dev_priv));
>
> + if (!intel_guc_is_loaded(guc))
> + return;
> +
And it would be good to consolidate uc_fini_hw into uc_sanitize where
uc_sanitize will look like:
void intel_uc_sanitize(struct drm_i915_private *i915)
{
struct intel_guc *guc = &i915->guc;
struct intel_huc *huc = &i915->huc;
if (USES_GUC(i915))
return;
GEM_BUG_ON(!HAS_GUC(i915));
if (!intel_guc_is_loaded(guc))
return;
if (USES_GUC_SUBMISSION(i915))
intel_guc_submission_disable(guc);
guc_disable_communication(guc);
if (USES_GUC_SUBMISSION(i915))
gen9_disable_guc_interrupts(i915);
intel_huc_sanitize(huc);
intel_guc_sanitize(guc);
__intel_uc_reset_hw(i915);
}
Then we can remove the call to intel_uc_fini_hw from i915_gem_fini.
This will also allow us to release the doorbells properly (calling
destroy_doorbell) as part of uc_sanitize.
> if (USES_GUC_SUBMISSION(dev_priv))
> intel_guc_submission_disable(guc);
>
> guc_disable_communication(guc);
> + intel_huc_sanitize(huc);
> + intel_guc_sanitize(guc);
>
> if (USES_GUC_SUBMISSION(dev_priv))
> gen9_disable_guc_interrupts(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
> index 2601521..ff9ce9a 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
> @@ -121,6 +121,11 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
> uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
> }
>
> +static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
> +{
> + return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
> +}
> +
> void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> struct intel_uc_fw *uc_fw);
> int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
--
Thanks,
Sagar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/4] drm/i915: Always do WOPCM partitioning based on real firmware sizes
@ 2018-04-10 0:42 Jackie Li
2018-04-10 0:42 ` [PATCH v3 4/4] HAX enable guc for CI Jackie Li
0 siblings, 1 reply; 9+ messages in thread
From: Jackie Li @ 2018-04-10 0:42 UTC (permalink / raw)
To: intel-gfx
After enabled the WOPCM write-once registers locking status checking,
reloading of the i915 module will fail with modparam enable_guc set to 3
(enable GuC and HuC firmware loading) if the module was originally loaded
with enable_guc set to 1 (only enable GuC firmware loading). This is
because WOPCM registers were updated and locked without considering the HuC
FW size. Since we need both GuC and HuC FW sizes to determine the final
layout of WOPCM, we should always calculate the WOPCM layout based on the
actual sizes of the GuC and HuC firmware available for a specific platform
if we need continue to support enable/disable HuC FW loading dynamically
with enable_guc modparam.
This patch splits uC firmware fetching into two stages. First stage is to
fetch the firmware image and verify the firmware header. uC firmware will
be marked as verified and this will make FW info available for following
WOPCM layout calculation. The second stage is to create a GEM object and
copy the FW data into the created GEM object which will only be available
when GuC/HuC loading is enabled by enable_guc modparam. This will guarantee
that the WOPCM layout will be always be calculated correctly without making
any assumptions to the GuC and HuC firmware sizes.
v3:
- Rebase
Signed-off-by: Jackie Li <yaodong.li@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/intel_uc.c | 14 ++++----------
drivers/gpu/drm/i915/intel_uc_fw.c | 31 ++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_uc_fw.h | 7 +++++--
3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1cffaf7..73b8f6c 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -172,11 +172,8 @@ void intel_uc_init_early(struct drm_i915_private *i915)
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);
+ intel_uc_fw_fetch(i915, &guc->fw, USES_GUC(i915));
+ intel_uc_fw_fetch(i915, &huc->fw, USES_HUC(i915));
}
void intel_uc_cleanup_early(struct drm_i915_private *i915)
@@ -184,11 +181,8 @@ 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);
+ intel_uc_fw_fini(&huc->fw);
+ intel_uc_fw_fini(&guc->fw);
guc_free_load_err_log(guc);
}
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 6e8e0b5..a9cb900 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -33,11 +33,13 @@
*
* @dev_priv: device private
* @uc_fw: uC firmware
+ * @copy_to_obj: whether fetch uC firmware into GEM object or not
*
- * Fetch uC firmware into GEM obj.
+ * Fetch and verify uC firmware and copy firmware data into GEM object if
+ * @copy_to_obj is true.
*/
void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
- struct intel_uc_fw *uc_fw)
+ struct intel_uc_fw *uc_fw, bool copy_to_obj)
{
struct pci_dev *pdev = dev_priv->drm.pdev;
struct drm_i915_gem_object *obj;
@@ -154,17 +156,24 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
goto fail;
}
- obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
- if (IS_ERR(obj)) {
- err = PTR_ERR(obj);
- DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
- intel_uc_fw_type_repr(uc_fw->type), err);
- goto fail;
+ uc_fw->size = fw->size;
+ uc_fw->fetch_status = INTEL_UC_FIRMWARE_VERIFIED;
+
+ if (copy_to_obj) {
+ obj = i915_gem_object_create_from_data(dev_priv, fw->data,
+ fw->size);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
+ intel_uc_fw_type_repr(uc_fw->type),
+ err);
+ goto fail;
+ }
+
+ uc_fw->obj = obj;
+ uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
}
- uc_fw->obj = obj;
- uc_fw->size = fw->size;
- uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
DRM_DEBUG_DRIVER("%s fw fetch %s\n",
intel_uc_fw_type_repr(uc_fw->type),
intel_uc_fw_status_repr(uc_fw->fetch_status));
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index dc33b12..4e7ecc8 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -36,6 +36,7 @@ enum intel_uc_fw_status {
INTEL_UC_FIRMWARE_FAIL = -1,
INTEL_UC_FIRMWARE_NONE = 0,
INTEL_UC_FIRMWARE_PENDING,
+ INTEL_UC_FIRMWARE_VERIFIED,
INTEL_UC_FIRMWARE_SUCCESS
};
@@ -84,6 +85,8 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
return "NONE";
case INTEL_UC_FIRMWARE_PENDING:
return "PENDING";
+ case INTEL_UC_FIRMWARE_VERIFIED:
+ return "VERIFIED";
case INTEL_UC_FIRMWARE_SUCCESS:
return "SUCCESS";
}
@@ -131,14 +134,14 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
*/
static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
{
- if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+ if (uc_fw->fetch_status < INTEL_UC_FIRMWARE_VERIFIED)
return 0;
return uc_fw->header_size + uc_fw->ucode_size;
}
void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
- struct intel_uc_fw *uc_fw);
+ struct intel_uc_fw *uc_fw, bool copy_to_obj);
int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
int (*xfer)(struct intel_uc_fw *uc_fw,
struct i915_vma *vma));
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 4/4] HAX enable guc for CI
2018-04-10 0:42 [PATCH v3 1/4] drm/i915: Always do WOPCM partitioning based on real firmware sizes Jackie Li
@ 2018-04-10 0:42 ` Jackie Li
0 siblings, 0 replies; 9+ messages in thread
From: Jackie Li @ 2018-04-10 0:42 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Jackie Li <yaodong.li@intel.com>
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c963603..53037b5 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@ struct drm_printer;
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc, 0) \
+ param(int, enable_guc, -1) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-04-10 0:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 13:54 [PATCH v3 1/4] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
2018-03-13 13:54 ` [PATCH v3 2/4] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
2018-03-14 6:49 ` Sagar Arun Kamble
2018-03-13 13:54 ` [PATCH v3 3/4] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
2018-03-14 6:57 ` Sagar Arun Kamble
2018-03-13 13:54 ` [PATCH v3 4/4] HAX: Enable GuC for CI Michal Wajdeczko
2018-03-13 15:45 ` ✗ Fi.CI.BAT: warning for series starting with [v3,1/4] drm/i915/uc: Use correct error code for GuC initialization failure Patchwork
2018-03-14 6:42 ` [PATCH v3 1/4] " Sagar Arun Kamble
2018-04-10 0:42 [PATCH v3 1/4] drm/i915: Always do WOPCM partitioning based on real firmware sizes Jackie Li
2018-04-10 0:42 ` [PATCH v3 4/4] HAX enable guc for CI Jackie Li
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.