* [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests
@ 2018-07-12 14:07 Jakub Bartmiński
2018-07-12 14:07 ` [PATCH 2/2] HAX enable GuC for CI Jakub Bartmiński
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Jakub Bartmiński @ 2018-07-12 14:07 UTC (permalink / raw)
To: intel-gfx; +Cc: Jakub Bartmiński
Some functions used within mock selftests may expect platform-dependent
automatic modparams parameters to have already been resolved, resulting
in failed assertions.
Backing up the modparams before mock selftests and manually setting
offending parameters inside the affected selftests should fix the issue.
Signed-off-by: Jakub Bartmiński <jakub.bartminski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_pci.c | 10 ++++++++++
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 6 ++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 55543f1b0236..9e41851f5ca2 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -733,11 +733,21 @@ static int __init i915_init(void)
{
bool use_kms = true;
int err;
+ struct i915_params *backup_modparams;
+
+ backup_modparams = kmalloc(sizeof(*backup_modparams), GFP_KERNEL);
+ if (!backup_modparams)
+ return -ENOMEM;
+ memcpy(backup_modparams, &i915_modparams, sizeof(*backup_modparams));
err = i915_mock_selftests();
if (err)
return err > 0 ? 0 : err;
+ /* Revert any modparams modifications made inside mock selftests */
+ memcpy(&i915_modparams, backup_modparams, sizeof(*backup_modparams));
+ kfree(backup_modparams);
+
/*
* Enable KMS by default, unless explicitly overriden by
* either the i915.modeset prarameter or by the
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index ab2590242033..de6aad832ac9 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -597,6 +597,12 @@ int i915_gem_context_mock_selftests(void)
if (!i915)
return -ENOMEM;
+ /*
+ * Platform defaults have not been resolved yet, so we need to prevent
+ * assertion failure on an unresolved enable_guc.
+ */
+ i915_modparams.enable_guc = 0;
+
err = i915_subtests(tests, i915);
drm_dev_put(&i915->drm);
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/2] HAX enable GuC for CI
2018-07-12 14:07 [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Jakub Bartmiński
@ 2018-07-12 14:07 ` Jakub Bartmiński
2018-07-12 14:26 ` [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Chris Wilson
2018-07-12 14:48 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2 siblings, 0 replies; 11+ messages in thread
From: Jakub Bartmiński @ 2018-07-12 14:07 UTC (permalink / raw)
To: intel-gfx
From: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@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 aebe0469ddaa..3e4e128237ac 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.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests
2018-07-12 14:07 [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Jakub Bartmiński
2018-07-12 14:07 ` [PATCH 2/2] HAX enable GuC for CI Jakub Bartmiński
@ 2018-07-12 14:26 ` Chris Wilson
2018-07-12 14:49 ` Daniel Vetter
2018-07-12 14:48 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-07-12 14:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Jakub Bartmiński
Quoting Jakub Bartmiński (2018-07-12 15:07:18)
> Some functions used within mock selftests may expect platform-dependent
> automatic modparams parameters to have already been resolved, resulting
> in failed assertions.
> Backing up the modparams before mock selftests and manually setting
> offending parameters inside the affected selftests should fix the issue.
I think hiding under a rock until it's weened off the modparam abuse and
can use a mock device is my favourite strategy here.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests
2018-07-12 14:26 ` [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Chris Wilson
@ 2018-07-12 14:49 ` Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2018-07-12 14:49 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Jakub Bartmiński
On Thu, Jul 12, 2018 at 03:26:41PM +0100, Chris Wilson wrote:
> Quoting Jakub Bartmiński (2018-07-12 15:07:18)
> > Some functions used within mock selftests may expect platform-dependent
> > automatic modparams parameters to have already been resolved, resulting
> > in failed assertions.
> > Backing up the modparams before mock selftests and manually setting
> > offending parameters inside the affected selftests should fix the issue.
>
> I think hiding under a rock until it's weened off the modparam abuse and
> can use a mock device is my favourite strategy here.
I think for reasons we want a local copy of the modparams in i915_dev.
Then we could sanitize/mock them at well in per-instances mocks even.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Keep local modparams copy for mock selftests
2018-07-12 14:07 [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Jakub Bartmiński
2018-07-12 14:07 ` [PATCH 2/2] HAX enable GuC for CI Jakub Bartmiński
2018-07-12 14:26 ` [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Chris Wilson
@ 2018-07-12 14:48 ` Patchwork
2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-07-12 14:48 UTC (permalink / raw)
To: Jakub Bartmiński; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Keep local modparams copy for mock selftests
URL : https://patchwork.freedesktop.org/series/46398/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9633 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/46398/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9633:
=== IGT changes ===
==== Possible regressions ====
igt@core_auth@basic-auth:
{fi-skl-iommu}: PASS -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_9633 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
== Participating hosts (45 -> 42) ==
Additional (1): fi-elk-e7500
Missing (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4475 -> Patchwork_9633
CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9633: 42e5990d61ded7af327247b74976c52a206e12d0 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
42e5990d61de HAX enable GuC for CI
ce9387fc7344 drm/i915: Keep local modparams copy for mock selftests
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9633/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] drm/i915/guc: Limit number of scratch registers used for H2G
@ 2018-10-16 9:42 Michal Wajdeczko
2018-10-16 9:42 ` [PATCH 2/2] HAX enable GuC for CI Michal Wajdeczko
0 siblings, 1 reply; 11+ messages in thread
From: Michal Wajdeczko @ 2018-10-16 9:42 UTC (permalink / raw)
To: intel-gfx
We wrongly assumed that GuC is only using last scratch register
for G2H messages, but in fact it is also using register [14] to
report sleep state status. Remove that register from our H2G
send registers pool.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_guc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 230aea6..34927d4e 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -50,7 +50,7 @@ void intel_guc_init_send_regs(struct intel_guc *guc)
unsigned int i;
guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
- guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+ guc->send_regs.count = SOFT_SCRATCH_COUNT - 2;
for (i = 0; i < guc->send_regs.count; i++) {
fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
--
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] 11+ messages in thread* [PATCH 2/2] HAX enable GuC for CI
2018-10-16 9:42 [PATCH 1/2] drm/i915/guc: Limit number of scratch registers used for H2G Michal Wajdeczko
@ 2018-10-16 9:42 ` Michal Wajdeczko
0 siblings, 0 replies; 11+ messages in thread
From: Michal Wajdeczko @ 2018-10-16 9:42 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@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 7e56c51..c681537 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -45,7 +45,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, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
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] 11+ messages in thread
* [PATCH 1/2] drm/i915/guc: Fix Gen9 GuC loading workarounds
@ 2018-10-16 8:59 Michal Wajdeczko
2018-10-16 8:59 ` [PATCH 2/2] HAX enable GuC for CI Michal Wajdeczko
0 siblings, 1 reply; 11+ messages in thread
From: Michal Wajdeczko @ 2018-10-16 8:59 UTC (permalink / raw)
To: intel-gfx
In commit 4502e9ec820d ("drm/i915/uc: Unify firmware loading") we
stopped converting errors detected during firmware transfer into
-EAGAIN and this indirectly killed our workarounds for Gen9 GuC.
Reactivate those workarounds by looking for actual -ETIMEDOUT error.
Testcase: igt@drv_selftest@live_hangcheck
Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
References: commit 4502e9ec820d ("drm/i915/uc: Unify firmware loading")
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_uc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index b1b3e81..b34c318 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -376,7 +376,7 @@ int intel_uc_init_hw(struct drm_i915_private *i915)
intel_guc_init_params(guc);
ret = intel_guc_fw_upload(guc);
- if (ret == 0 || ret != -EAGAIN)
+ if (ret == 0 || ret != -ETIMEDOUT)
break;
DRM_DEBUG_DRIVER("GuC fw load failed: %d; will reset and "
--
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] 11+ messages in thread* [PATCH 2/2] HAX enable GuC for CI
2018-10-16 8:59 [PATCH 1/2] drm/i915/guc: Fix Gen9 GuC loading workarounds Michal Wajdeczko
@ 2018-10-16 8:59 ` Michal Wajdeczko
0 siblings, 0 replies; 11+ messages in thread
From: Michal Wajdeczko @ 2018-10-16 8:59 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@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 7e56c51..c681537 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -45,7 +45,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, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
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] 11+ messages in thread
* [PATCH 1/2] drm/i915/guc: fix GuC suspend/resume
@ 2018-10-15 22:10 Daniele Ceraolo Spurio
2018-10-15 22:10 ` [PATCH 2/2] HAX enable GuC for CI Daniele Ceraolo Spurio
0 siblings, 1 reply; 11+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-15 22:10 UTC (permalink / raw)
To: intel-gfx
The ENTER/EXIT_S_STATE actions queue the save/restore operation in GuC
FW and then return, so waiting on the H2H is not enough to guarantee
GuC is done.
When all the processing is done, GuC writes 0 to scratch register 14,
so we can poll on that. Note that GuC does not ensure that the value
in the register is different from 0 while the action is in progress
so we need to take care of that ourselves as well.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/intel_guc.c | 28 +++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_guc_fwif.h | 6 ++++++
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 230aea69385d..f238cd7a9dcf 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -521,6 +521,30 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
return intel_guc_send(guc, action, ARRAY_SIZE(action));
}
+/*
+ * The ENTER/EXIT_S_STATE actions queue the save/restore operation in GuC FW and
+ * then return, so waiting on the H2H is not enough to guarantee GuC is done.
+ * When all the processing is done, GuC writes 0 to scratch register 14, so we
+ * can poll on that. Note that GuC does not ensure that the value in the
+ * register is different from 0 while the action is in progress so we need to
+ * take care of that ourselves as well.
+ */
+static int guc_sleep_state_action(struct intel_guc *guc,
+ const u32 *action, u32 len)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ int ret;
+
+ I915_WRITE(SOFT_SCRATCH(14), ~0x0);
+
+ ret = intel_guc_send(guc, action, len);
+ if (ret)
+ return ret;
+
+ return intel_wait_for_register(dev_priv, SOFT_SCRATCH(14), ~0x0,
+ INTEL_GUC_SLEEP_STATE_SUCCESS, 10);
+}
+
/**
* intel_guc_suspend() - notify GuC entering suspend state
* @guc: the guc
@@ -533,7 +557,7 @@ int intel_guc_suspend(struct intel_guc *guc)
intel_guc_ggtt_offset(guc, guc->shared_data)
};
- return intel_guc_send(guc, data, ARRAY_SIZE(data));
+ return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
}
/**
@@ -571,7 +595,7 @@ int intel_guc_resume(struct intel_guc *guc)
intel_guc_ggtt_offset(guc, guc->shared_data)
};
- return intel_guc_send(guc, data, ARRAY_SIZE(data));
+ return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
}
/**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 8382d591c784..b0eb5aabe0a7 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -687,6 +687,12 @@ enum intel_guc_report_status {
INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
};
+enum intel_guc_sleep_state_status {
+ INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
+ INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
+ INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
+};
+
#define GUC_LOG_CONTROL_LOGGING_ENABLED (1 << 0)
#define GUC_LOG_CONTROL_VERBOSITY_SHIFT 4
#define GUC_LOG_CONTROL_VERBOSITY_MASK (0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT)
--
2.19.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/2] HAX enable GuC for CI
2018-10-15 22:10 [PATCH 1/2] drm/i915/guc: fix GuC suspend/resume Daniele Ceraolo Spurio
@ 2018-10-15 22:10 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 11+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-10-15 22:10 UTC (permalink / raw)
To: intel-gfx
From: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@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 7e56c516c815..c681537bcb92 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -45,7 +45,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.19.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/2] drm/i915: Add code to accept valid locked WOPCM register values
@ 2018-03-20 23:18 Jackie Li
2018-03-20 23:18 ` [PATCH 2/2] HAX enable guc for CI Jackie Li
0 siblings, 1 reply; 11+ messages in thread
From: Jackie Li @ 2018-03-20 23:18 UTC (permalink / raw)
To: intel-gfx
In current code, we only compare the locked WOPCM register values with the
calculated values. However, we can continue loading GuC/HuC firmware if the
locked (or partially locked) values were valid for current GuC/HuC firmware
sizes.
This patch added a new code path to verify whether the locked register
values can be used for GuC/HuC firmware loading, it will recalculate the
verify the new values if these registers were partially locked, so that we
won't fail the GuC/HuC firmware loading even if the locked register values
are different from the calculated ones.
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: Michał Winiarski <michal.winiarski@intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
---
drivers/gpu/drm/i915/intel_wopcm.c | 183 +++++++++++++++++++++++++++++++------
1 file changed, 157 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 4117886..fbd8aba 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -138,6 +138,53 @@ static inline int check_hw_restriction(struct drm_i915_private *i915,
return err;
}
+static inline u32 calculate_min_guc_wopcm_base(u32 huc_fw_size)
+{
+ return ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
+ GUC_WOPCM_OFFSET_ALIGNMENT);
+}
+
+static inline u32 calculate_min_guc_wopcm_size(u32 guc_fw_size)
+{
+ return guc_fw_size + GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
+}
+
+static inline int calculate_max_guc_wopcm_size(struct intel_wopcm *wopcm,
+ u32 guc_wopcm_base,
+ u32 *guc_wopcm_size)
+{
+ struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
+ u32 ctx_rsvd = context_reserved_size(i915);
+
+ if (guc_wopcm_base >= wopcm->size ||
+ (guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
+ DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
+ guc_wopcm_base / 1024);
+ return -E2BIG;
+ }
+
+ *guc_wopcm_size =
+ (wopcm->size - guc_wopcm_base - ctx_rsvd) & GUC_WOPCM_SIZE_MASK;
+
+ return 0;
+}
+
+static inline int verify_calculated_values(struct drm_i915_private *i915,
+ u32 guc_fw_size, u32 huc_fw_size,
+ u32 guc_wopcm_base,
+ u32 guc_wopcm_size)
+{
+ if (guc_wopcm_size < calculate_min_guc_wopcm_size(guc_fw_size)) {
+ DRM_ERROR("Need %uKiB WOPCM for GuC FW, %uKiB available.\n",
+ calculate_min_guc_wopcm_size(guc_fw_size),
+ guc_wopcm_size / 1024);
+ return -E2BIG;
+ }
+
+ return check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size,
+ huc_fw_size);
+}
+
/**
* intel_wopcm_init() - Initialize the WOPCM structure.
* @wopcm: pointer to intel_wopcm.
@@ -155,10 +202,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
u32 guc_fw_size = intel_uc_fw_get_upload_size(&i915->guc.fw);
u32 huc_fw_size = intel_uc_fw_get_upload_size(&i915->huc.fw);
- u32 ctx_rsvd = context_reserved_size(i915);
u32 guc_wopcm_base;
u32 guc_wopcm_size;
- u32 guc_wopcm_rsvd;
int err;
GEM_BUG_ON(!wopcm->size);
@@ -175,30 +220,17 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
return -E2BIG;
}
- guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
- GUC_WOPCM_OFFSET_ALIGNMENT);
- if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
- DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
- guc_wopcm_base / 1024);
- return -E2BIG;
- }
-
- guc_wopcm_size = wopcm->size - guc_wopcm_base - ctx_rsvd;
- guc_wopcm_size &= GUC_WOPCM_SIZE_MASK;
+ guc_wopcm_base = calculate_min_guc_wopcm_base(huc_fw_size);
+ err = calculate_max_guc_wopcm_size(wopcm, guc_wopcm_base,
+ &guc_wopcm_size);
+ if (err)
+ return err;
DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB, %uKiB)\n",
guc_wopcm_base / 1024, guc_wopcm_size / 1024);
- guc_wopcm_rsvd = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
- if ((guc_fw_size + guc_wopcm_rsvd) > guc_wopcm_size) {
- DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
- (guc_fw_size + guc_wopcm_rsvd) / 1024,
- guc_wopcm_size / 1024);
- return -E2BIG;
- }
-
- err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size,
- huc_fw_size);
+ err = verify_calculated_values(i915, guc_fw_size, huc_fw_size,
+ guc_wopcm_base, guc_wopcm_size);
if (err)
return err;
@@ -208,6 +240,92 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
return 0;
}
+static inline int verify_locked_values(struct intel_wopcm *wopcm,
+ u32 guc_wopcm_base, u32 guc_wopcm_size)
+{
+ struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
+ u32 guc_fw_size = intel_uc_fw_get_upload_size(&i915->guc.fw);
+ u32 huc_fw_size = intel_uc_fw_get_upload_size(&i915->huc.fw);
+ u32 ctx_rsvd = context_reserved_size(i915);
+
+ /*
+ * Locked GuC WOPCM base and size could be any values are large enough
+ * to lead to a wrap around after performing add operations.
+ */
+ if (guc_wopcm_base >= wopcm->size) {
+ DRM_ERROR("Locked base value %uKiB >= WOPCM size %uKiB.\n",
+ guc_wopcm_base / 1024,
+ wopcm->size / 1024);
+ return -E2BIG;
+ }
+
+ if (guc_wopcm_size >= wopcm->size) {
+ DRM_ERROR("Locked size value %uKiB >= WOPCM size %uKiB.\n",
+ guc_wopcm_base / 1024,
+ wopcm->size / 1024);
+ return -E2BIG;
+ }
+
+ if (guc_wopcm_base + guc_wopcm_size + ctx_rsvd > wopcm->size) {
+ DRM_ERROR("Need %uKiB WOPCM in total, %uKiB available.\n",
+ (guc_wopcm_base + guc_wopcm_size + ctx_rsvd) / 1024,
+ (wopcm->size) / 1024);
+ return -E2BIG;
+ }
+
+ if (guc_wopcm_base < calculate_min_guc_wopcm_base(huc_fw_size)) {
+ DRM_ERROR("Need %uKiB WOPCM for HuC FW, %uKiB available.\n",
+ calculate_min_guc_wopcm_base(huc_fw_size) / 1024,
+ guc_wopcm_base / 1024);
+ return -E2BIG;
+ }
+
+ return verify_calculated_values(i915, guc_fw_size, huc_fw_size,
+ guc_wopcm_base, guc_wopcm_size);
+}
+
+static inline int verify_locked_values_and_update(struct intel_wopcm *wopcm)
+{
+ struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
+ u32 huc_fw_size = intel_uc_fw_get_upload_size(&dev_priv->huc.fw);
+ u32 size_val = I915_READ(GUC_WOPCM_SIZE);
+ u32 offset_val = I915_READ(DMA_GUC_WOPCM_OFFSET);
+ bool offset_reg_locked = offset_val & GUC_WOPCM_OFFSET_VALID;
+ bool size_reg_locked = size_val & GUC_WOPCM_SIZE_LOCKED;
+ u32 guc_wopcm_base = offset_val & GUC_WOPCM_OFFSET_MASK;
+ u32 guc_wopcm_size = size_val & GUC_WOPCM_SIZE_MASK;
+ int err;
+
+ if (!offset_reg_locked && !size_reg_locked)
+ return 0;
+
+ if (guc_wopcm_base == wopcm->guc.base &&
+ guc_wopcm_size == wopcm->guc.size)
+ return 0;
+
+ if (!offset_reg_locked)
+ guc_wopcm_base = calculate_min_guc_wopcm_base(huc_fw_size);
+
+ if (!size_reg_locked) {
+ err = calculate_max_guc_wopcm_size(wopcm, guc_wopcm_base,
+ &guc_wopcm_size);
+ if (err)
+ return err;
+ }
+
+ DRM_DEBUG_DRIVER("Recalculated GuC WOPCM Region: [%uKiB, %uKiB)\n",
+ guc_wopcm_base / 1024, guc_wopcm_size / 1024);
+
+ err = verify_locked_values(wopcm, guc_wopcm_base, guc_wopcm_size);
+ if (err)
+ return err;
+
+ wopcm->guc.size = guc_wopcm_size;
+ wopcm->guc.base = guc_wopcm_base;
+
+ return 0;
+}
+
static inline int write_and_verify(struct drm_i915_private *dev_priv,
i915_reg_t reg, u32 val, u32 mask,
u32 locked_bit)
@@ -231,7 +349,8 @@ static inline int write_and_verify(struct drm_i915_private *dev_priv,
* will verify the register values to make sure the registers are locked with
* correct values.
*
- * Return: 0 on success. -EIO if registers were locked with incorrect values.
+ * Return: 0 on success. Minus error code if registered were locked with
+ * incorrect values.-EIO if registers failed to lock with correct values.
*/
int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
{
@@ -247,27 +366,39 @@ int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
GEM_BUG_ON(!wopcm->guc.size);
GEM_BUG_ON(!wopcm->guc.base);
+ err = verify_locked_values_and_update(wopcm);
+ if (err) {
+ DRM_ERROR("WOPCM registers are locked with invalid values.\n");
+ goto err_out;
+ }
+
err = write_and_verify(dev_priv, GUC_WOPCM_SIZE, wopcm->guc.size,
GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED,
GUC_WOPCM_SIZE_LOCKED);
- if (err)
+ if (err) {
+ DRM_ERROR("Failed to lock GUC_WOPCM_SIZE with %#x.\n",
+ wopcm->guc.size);
goto err_out;
+ }
huc_agent = USES_HUC(dev_priv) ? HUC_LOADING_AGENT_GUC : 0;
mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
wopcm->guc.base | huc_agent, mask,
GUC_WOPCM_OFFSET_VALID);
- if (err)
+ if (err) {
+ DRM_ERROR("Failed to lock DMA_GUC_WOPCM_OFFSET with %#x.\n",
+ wopcm->guc.base);
goto err_out;
+ }
return 0;
err_out:
- DRM_ERROR("Failed to init WOPCM registers:\n");
DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
I915_READ(DMA_GUC_WOPCM_OFFSET));
DRM_ERROR("GUC_WOPCM_SIZE=%#x\n", I915_READ(GUC_WOPCM_SIZE));
+ DRM_ERROR("A system reboot is required.\n");
return err;
}
--
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] 11+ messages in thread* [PATCH 2/2] HAX enable guc for CI
2018-03-20 23:18 [PATCH 1/2] drm/i915: Add code to accept valid locked WOPCM register values Jackie Li
@ 2018-03-20 23:18 ` Jackie Li
0 siblings, 0 replies; 11+ messages in thread
From: Jackie Li @ 2018-03-20 23:18 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] 11+ messages in thread
* [PATCH 1/2] drm/i915/uc: Introduce intel_uc_suspend|resume
@ 2018-02-23 14:10 Michal Wajdeczko
2018-02-23 14:10 ` [PATCH 2/2] HAX: Enable GuC for CI Michal Wajdeczko
0 siblings, 1 reply; 11+ messages in thread
From: Michal Wajdeczko @ 2018-02-23 14:10 UTC (permalink / raw)
To: intel-gfx
We want to use higher level 'uc' functions as the main entry points to
the GuC/HuC code to hide some details and keep code layered.
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/i915_drv.c | 6 +++---
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
drivers/gpu/drm/i915/intel_guc.c | 42 +++++++++++++------------------------
drivers/gpu/drm/i915/intel_guc.h | 4 ++--
drivers/gpu/drm/i915/intel_uc.c | 45 ++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_uc.h | 2 ++
6 files changed, 68 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index aaa861b..d61b51c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2575,7 +2575,7 @@ static int intel_runtime_suspend(struct device *kdev)
*/
i915_gem_runtime_suspend(dev_priv);
- intel_guc_suspend(dev_priv);
+ intel_uc_suspend(dev_priv);
intel_runtime_pm_disable_interrupts(dev_priv);
@@ -2597,7 +2597,7 @@ static int intel_runtime_suspend(struct device *kdev)
intel_runtime_pm_enable_interrupts(dev_priv);
- intel_guc_resume(dev_priv);
+ intel_uc_resume(dev_priv);
i915_gem_init_swizzling(dev_priv);
i915_gem_restore_fences(dev_priv);
@@ -2683,7 +2683,7 @@ static int intel_runtime_resume(struct device *kdev)
intel_runtime_pm_enable_interrupts(dev_priv);
- intel_guc_resume(dev_priv);
+ intel_uc_resume(dev_priv);
/*
* No point of rolling back things in case of an error, as the best
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 14c855b..ef6aff8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4919,7 +4919,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
i915_gem_contexts_lost(dev_priv);
mutex_unlock(&dev->struct_mutex);
- intel_guc_suspend(dev_priv);
+ intel_uc_suspend(dev_priv);
cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
cancel_delayed_work_sync(&dev_priv->gt.retire_work);
@@ -4986,7 +4986,7 @@ void i915_gem_resume(struct drm_i915_private *i915)
if (i915_gem_init_hw(i915))
goto err_wedged;
- intel_guc_resume(i915);
+ intel_uc_resume(i915);
/* Always reload a context for powersaving. */
if (i915_gem_switch_to_kernel_context(i915))
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 21140cc..6ca6b9a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -403,22 +403,15 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
/**
* intel_guc_suspend() - notify GuC entering suspend state
- * @dev_priv: i915 device private
+ * @guc: the guc
*/
-int intel_guc_suspend(struct drm_i915_private *dev_priv)
+int intel_guc_suspend(struct intel_guc *guc)
{
- struct intel_guc *guc = &dev_priv->guc;
- u32 data[3];
-
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
- return 0;
-
- gen9_disable_guc_interrupts(dev_priv);
-
- data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
- /* any value greater than GUC_POWER_D0 */
- data[1] = GUC_POWER_D1;
- data[2] = guc_ggtt_offset(guc->shared_data);
+ u32 data[] = {
+ INTEL_GUC_ACTION_ENTER_S_STATE,
+ GUC_POWER_D1, /* any value greater than GUC_POWER_D0 */
+ guc_ggtt_offset(guc->shared_data)
+ };
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
@@ -448,22 +441,15 @@ int intel_guc_reset_engine(struct intel_guc *guc,
/**
* intel_guc_resume() - notify GuC resuming from suspend state
- * @dev_priv: i915 device private
+ * @guc: the guc
*/
-int intel_guc_resume(struct drm_i915_private *dev_priv)
+int intel_guc_resume(struct intel_guc *guc)
{
- struct intel_guc *guc = &dev_priv->guc;
- u32 data[3];
-
- if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
- return 0;
-
- if (i915_modparams.guc_log_level)
- gen9_enable_guc_interrupts(dev_priv);
-
- data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
- data[1] = GUC_POWER_D0;
- data[2] = guc_ggtt_offset(guc->shared_data);
+ u32 data[] = {
+ INTEL_GUC_ACTION_EXIT_S_STATE,
+ GUC_POWER_D0,
+ guc_ggtt_offset(guc->shared_data)
+ };
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 52856a9..b9424ac 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -127,8 +127,8 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
int intel_guc_sample_forcewake(struct intel_guc *guc);
int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
-int intel_guc_suspend(struct drm_i915_private *dev_priv);
-int intel_guc_resume(struct drm_i915_private *dev_priv);
+int intel_guc_suspend(struct intel_guc *guc);
+int intel_guc_resume(struct intel_guc *guc);
struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9f1bac6..a821f7a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -445,3 +445,48 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
if (USES_GUC_SUBMISSION(dev_priv))
gen9_disable_guc_interrupts(dev_priv);
}
+
+int intel_uc_suspend(struct drm_i915_private *i915)
+{
+ struct intel_guc *guc = &i915->guc;
+ int err;
+
+ if (!USES_GUC(i915))
+ return 0;
+
+ if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ return 0;
+
+ err = intel_guc_suspend(guc);
+ if (err) {
+ DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
+ return err;
+ }
+
+ gen9_disable_guc_interrupts(i915);
+
+ return 0;
+}
+
+int intel_uc_resume(struct drm_i915_private *i915)
+{
+ struct intel_guc *guc = &i915->guc;
+ int err;
+
+ if (!USES_GUC(i915))
+ return 0;
+
+ if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+ return 0;
+
+ if (i915_modparams.guc_log_level)
+ gen9_enable_guc_interrupts(i915);
+
+ err = intel_guc_resume(guc);
+ if (err) {
+ DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
+ return err;
+ }
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f2984e0..f76d51d 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -39,6 +39,8 @@
void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
int intel_uc_init(struct drm_i915_private *dev_priv);
void intel_uc_fini(struct drm_i915_private *dev_priv);
+int intel_uc_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_resume(struct drm_i915_private *dev_priv);
static inline bool intel_uc_is_using_guc(void)
{
--
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] 11+ messages in thread* [PATCH 2/2] HAX: Enable GuC for CI
2018-02-23 14:10 [PATCH 1/2] drm/i915/uc: Introduce intel_uc_suspend|resume Michal Wajdeczko
@ 2018-02-23 14:10 ` Michal Wajdeczko
0 siblings, 0 replies; 11+ messages in thread
From: Michal Wajdeczko @ 2018-02-23 14:10 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 a821f7a..0e72e60 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 *dev_priv)
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] 11+ messages in thread
* [PATCH 1/2] drm/i915/preemption: Allow preemption between submission ports
@ 2018-02-23 9:04 Chris Wilson
2018-02-23 9:04 ` [PATCH 2/2] HAX: enable GuC for CI Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-02-23 9:04 UTC (permalink / raw)
To: intel-gfx
Sometimes we need to boost the priority of an in-flight request, which
may lead to the situation where the second submission port then contains
a higher priority context than the first and so we need to inject a
preemption event. To do so we must always check inside
execlists_dequeue() whether there is a priority inversion between the
ports themselves as well as the head of the priority sorted queue, and we
cannot just skip dequeuing if the queue is empty.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_engine_cs.c | 2 +
drivers/gpu/drm/i915/intel_guc_submission.c | 17 +--
drivers/gpu/drm/i915/intel_lrc.c | 161 ++++++++++++++++------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +
4 files changed, 107 insertions(+), 78 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index c31544406974..ce7fcf55ba18 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -423,6 +423,7 @@ static void intel_engine_init_execlist(struct intel_engine_cs *engine)
BUILD_BUG_ON_NOT_POWER_OF_2(execlists_num_ports(execlists));
GEM_BUG_ON(execlists_num_ports(execlists) > EXECLIST_MAX_PORTS);
+ execlists->queue_priority = INT_MIN;
execlists->queue = RB_ROOT;
execlists->first = NULL;
}
@@ -1903,6 +1904,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
spin_lock_irq(&engine->timeline->lock);
list_for_each_entry(rq, &engine->timeline->requests, link)
print_request(m, rq, "\t\tE ");
+ drm_printf(m, "\t\tQueue priority: %d\n", execlists->queue_priority);
for (rb = execlists->first; rb; rb = rb_next(rb)) {
struct i915_priolist *p =
rb_entry(rb, typeof(*p), node);
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 649113c7a3c2..586dde579903 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -75,6 +75,11 @@
*
*/
+static inline struct i915_priolist *to_priolist(struct rb_node *rb)
+{
+ return rb_entry(rb, struct i915_priolist, node);
+}
+
static inline bool is_high_priority(struct intel_guc_client *client)
{
return (client->priority == GUC_CLIENT_PRIORITY_KMD_HIGH ||
@@ -682,15 +687,12 @@ static void guc_dequeue(struct intel_engine_cs *engine)
rb = execlists->first;
GEM_BUG_ON(rb_first(&execlists->queue) != rb);
- if (!rb)
- goto unlock;
-
if (port_isset(port)) {
if (engine->i915->preempt_context) {
struct guc_preempt_work *preempt_work =
&engine->i915->guc.preempt_work[engine->id];
- if (rb_entry(rb, struct i915_priolist, node)->priority >
+ if (execlists->queue_priority >
max(port_request(port)->priotree.priority, 0)) {
execlists_set_active(execlists,
EXECLISTS_ACTIVE_PREEMPT);
@@ -706,8 +708,8 @@ static void guc_dequeue(struct intel_engine_cs *engine)
}
GEM_BUG_ON(port_isset(port));
- do {
- struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
+ while (rb) {
+ struct i915_priolist *p = to_priolist(rb);
struct i915_request *rq, *rn;
list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) {
@@ -736,8 +738,9 @@ static void guc_dequeue(struct intel_engine_cs *engine)
INIT_LIST_HEAD(&p->requests);
if (p->priority != I915_PRIORITY_NORMAL)
kmem_cache_free(engine->i915->priorities, p);
- } while (rb);
+ }
done:
+ execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN;
execlists->first = rb;
if (submit) {
port_assign(port, last);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 964885b5d7cb..4bc72fbaf793 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -169,6 +169,23 @@ static void execlists_init_reg_state(u32 *reg_state,
struct intel_engine_cs *engine,
struct intel_ring *ring);
+static inline struct i915_priolist *to_priolist(struct rb_node *rb)
+{
+ return rb_entry(rb, struct i915_priolist, node);
+}
+
+static inline int rq_prio(const struct i915_request *rq)
+{
+ return rq->priotree.priority;
+}
+
+static inline bool need_preempt(const struct intel_engine_cs *engine,
+ const struct i915_request *last,
+ int prio)
+{
+ return engine->i915->preempt_context && prio > max(rq_prio(last), 0);
+}
+
/**
* intel_lr_context_descriptor_update() - calculate & cache the descriptor
* descriptor for a pinned context
@@ -224,7 +241,7 @@ lookup_priolist(struct intel_engine_cs *engine,
parent = &execlists->queue.rb_node;
while (*parent) {
rb = *parent;
- p = rb_entry(rb, typeof(*p), node);
+ p = to_priolist(rb);
if (prio > p->priority) {
parent = &rb->rb_left;
} else if (prio < p->priority) {
@@ -264,7 +281,7 @@ lookup_priolist(struct intel_engine_cs *engine,
if (first)
execlists->first = &p->node;
- return ptr_pack_bits(p, first, 1);
+ return p;
}
static void unwind_wa_tail(struct i915_request *rq)
@@ -290,14 +307,10 @@ static void __unwind_incomplete_requests(struct intel_engine_cs *engine)
__i915_request_unsubmit(rq);
unwind_wa_tail(rq);
- GEM_BUG_ON(rq->priotree.priority == I915_PRIORITY_INVALID);
- if (rq->priotree.priority != last_prio) {
- p = lookup_priolist(engine,
- &rq->priotree,
- rq->priotree.priority);
- p = ptr_mask_bits(p, 1);
-
- last_prio = rq->priotree.priority;
+ GEM_BUG_ON(rq_prio(rq) == I915_PRIORITY_INVALID);
+ if (rq_prio(rq) != last_prio) {
+ last_prio = rq_prio(rq);
+ p = lookup_priolist(engine, &rq->priotree, last_prio);
}
list_add(&rq->priotree.link, &p->requests);
@@ -397,10 +410,11 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
desc = execlists_update_context(rq);
GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc));
- GEM_TRACE("%s in[%d]: ctx=%d.%d, seqno=%x\n",
+ GEM_TRACE("%s in[%d]: ctx=%d.%d, seqno=%x, prio=%d\n",
engine->name, n,
port[n].context_id, count,
- rq->global_seqno);
+ rq->global_seqno,
+ rq_prio(rq));
} else {
GEM_BUG_ON(!n);
desc = 0;
@@ -453,12 +467,17 @@ static void inject_preempt_context(struct intel_engine_cs *engine)
_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT));
+ /*
+ * Switch to our empty preempt context so
+ * the state of the GPU is known (idle).
+ */
GEM_TRACE("%s\n", engine->name);
for (n = execlists_num_ports(&engine->execlists); --n; )
elsp_write(0, engine->execlists.elsp);
elsp_write(ce->lrc_desc, engine->execlists.elsp);
execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK);
+ execlists_set_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT);
}
static void execlists_dequeue(struct intel_engine_cs *engine)
@@ -495,8 +514,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
spin_lock_irq(&engine->timeline->lock);
rb = execlists->first;
GEM_BUG_ON(rb_first(&execlists->queue) != rb);
- if (!rb)
- goto unlock;
if (last) {
/*
@@ -519,54 +536,48 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_HWACK))
goto unlock;
- if (engine->i915->preempt_context &&
- rb_entry(rb, struct i915_priolist, node)->priority >
- max(last->priotree.priority, 0)) {
- /*
- * Switch to our empty preempt context so
- * the state of the GPU is known (idle).
- */
+ if (need_preempt(engine, last, execlists->queue_priority)) {
inject_preempt_context(engine);
- execlists_set_active(execlists,
- EXECLISTS_ACTIVE_PREEMPT);
goto unlock;
- } else {
- /*
- * In theory, we could coalesce more requests onto
- * the second port (the first port is active, with
- * no preemptions pending). However, that means we
- * then have to deal with the possible lite-restore
- * of the second port (as we submit the ELSP, there
- * may be a context-switch) but also we may complete
- * the resubmission before the context-switch. Ergo,
- * coalescing onto the second port will cause a
- * preemption event, but we cannot predict whether
- * that will affect port[0] or port[1].
- *
- * If the second port is already active, we can wait
- * until the next context-switch before contemplating
- * new requests. The GPU will be busy and we should be
- * able to resubmit the new ELSP before it idles,
- * avoiding pipeline bubbles (momentary pauses where
- * the driver is unable to keep up the supply of new
- * work).
- */
- if (port_count(&port[1]))
- goto unlock;
-
- /* WaIdleLiteRestore:bdw,skl
- * Apply the wa NOOPs to prevent
- * ring:HEAD == rq:TAIL as we resubmit the
- * request. See gen8_emit_breadcrumb() for
- * where we prepare the padding after the
- * end of the request.
- */
- last->tail = last->wa_tail;
}
+
+ /*
+ * In theory, we could coalesce more requests onto
+ * the second port (the first port is active, with
+ * no preemptions pending). However, that means we
+ * then have to deal with the possible lite-restore
+ * of the second port (as we submit the ELSP, there
+ * may be a context-switch) but also we may complete
+ * the resubmission before the context-switch. Ergo,
+ * coalescing onto the second port will cause a
+ * preemption event, but we cannot predict whether
+ * that will affect port[0] or port[1].
+ *
+ * If the second port is already active, we can wait
+ * until the next context-switch before contemplating
+ * new requests. The GPU will be busy and we should be
+ * able to resubmit the new ELSP before it idles,
+ * avoiding pipeline bubbles (momentary pauses where
+ * the driver is unable to keep up the supply of new
+ * work). However, we have to double check that the
+ * priorities of the ports haven't been switch.
+ */
+ if (port_count(&port[1]))
+ goto unlock;
+
+ /*
+ * WaIdleLiteRestore:bdw,skl
+ * Apply the wa NOOPs to prevent
+ * ring:HEAD == rq:TAIL as we resubmit the
+ * request. See gen8_emit_breadcrumb() for
+ * where we prepare the padding after the
+ * end of the request.
+ */
+ last->tail = last->wa_tail;
}
- do {
- struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
+ while (rb) {
+ struct i915_priolist *p = to_priolist(rb);
struct i915_request *rq, *rn;
list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) {
@@ -628,8 +639,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
INIT_LIST_HEAD(&p->requests);
if (p->priority != I915_PRIORITY_NORMAL)
kmem_cache_free(engine->i915->priorities, p);
- } while (rb);
+ }
done:
+ execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN;
execlists->first = rb;
if (submit)
port_assign(port, last);
@@ -690,7 +702,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
/* Flush the queued requests to the timeline list (for retiring). */
rb = execlists->first;
while (rb) {
- struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
+ struct i915_priolist *p = to_priolist(rb);
list_for_each_entry_safe(rq, rn, &p->requests, priotree.link) {
INIT_LIST_HEAD(&rq->priotree.link);
@@ -708,7 +720,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
/* Remaining _unready_ requests will be nop'ed when submitted */
-
+ execlists->queue_priority = INT_MIN;
execlists->queue = RB_ROOT;
execlists->first = NULL;
GEM_BUG_ON(port_isset(execlists->port));
@@ -864,10 +876,11 @@ static void execlists_submission_tasklet(unsigned long data)
EXECLISTS_ACTIVE_USER));
rq = port_unpack(port, &count);
- GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%x\n",
+ GEM_TRACE("%s out[0]: ctx=%d.%d, seqno=%x, prio=%d\n",
engine->name,
port->context_id, count,
- rq ? rq->global_seqno : 0);
+ rq ? rq->global_seqno : 0,
+ rq ? rq_prio(rq) : 0);
/* Check the context/desc id for this event matches */
GEM_DEBUG_BUG_ON(buf[2 * head + 1] != port->context_id);
@@ -912,15 +925,19 @@ static void execlists_submission_tasklet(unsigned long data)
intel_uncore_forcewake_put(dev_priv, execlists->fw_domains);
}
-static void insert_request(struct intel_engine_cs *engine,
- struct i915_priotree *pt,
- int prio)
+static void queue_request(struct intel_engine_cs *engine,
+ struct i915_priotree *pt,
+ int prio)
{
- struct i915_priolist *p = lookup_priolist(engine, pt, prio);
+ list_add_tail(&pt->link, &lookup_priolist(engine, pt, prio)->requests);
+}
- list_add_tail(&pt->link, &ptr_mask_bits(p, 1)->requests);
- if (ptr_unmask_bits(p, 1))
+static void submit_queue(struct intel_engine_cs *engine, int prio)
+{
+ if (prio > engine->execlists.queue_priority) {
+ engine->execlists.queue_priority = prio;
tasklet_hi_schedule(&engine->execlists.tasklet);
+ }
}
static void execlists_submit_request(struct i915_request *request)
@@ -931,7 +948,8 @@ static void execlists_submit_request(struct i915_request *request)
/* Will be called from irq-context when using foreign fences. */
spin_lock_irqsave(&engine->timeline->lock, flags);
- insert_request(engine, &request->priotree, request->priotree.priority);
+ queue_request(engine, &request->priotree, rq_prio(request));
+ submit_queue(engine, rq_prio(request));
GEM_BUG_ON(!engine->execlists.first);
GEM_BUG_ON(list_empty(&request->priotree.link));
@@ -987,7 +1005,7 @@ static void execlists_schedule(struct i915_request *request, int prio)
* static void update_priorities(struct i915_priotree *pt, prio) {
* list_for_each_entry(dep, &pt->signalers_list, signal_link)
* update_priorities(dep->signal, prio)
- * insert_request(pt);
+ * queue_request(pt);
* }
* but that may have unlimited recursion depth and so runs a very
* real risk of overunning the kernel stack. Instead, we build
@@ -1050,8 +1068,9 @@ static void execlists_schedule(struct i915_request *request, int prio)
pt->priority = prio;
if (!list_empty(&pt->link)) {
__list_del_entry(&pt->link);
- insert_request(engine, pt, prio);
+ queue_request(engine, pt, prio);
}
+ submit_queue(engine, prio);
}
spin_unlock_irq(&engine->timeline->lock);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a9b83bf7e837..c4e9022b34e3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -257,6 +257,11 @@ struct intel_engine_execlists {
*/
unsigned int port_mask;
+ /**
+ * @queue_priority: Highest pending priority.
+ */
+ int queue_priority;
+
/**
* @queue: queue of requests, in priority lists
*/
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/2] HAX: enable GuC for CI
2018-02-23 9:04 [PATCH 1/2] drm/i915/preemption: Allow preemption between submission ports Chris Wilson
@ 2018-02-23 9:04 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2018-02-23 9:04 UTC (permalink / raw)
To: intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@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 430f5f9d0ff4..3deae1e22974 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, 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 9f1bac6398fb..b48056fb769d 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 *dev_priv)
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;
}
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-16 9:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-12 14:07 [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Jakub Bartmiński
2018-07-12 14:07 ` [PATCH 2/2] HAX enable GuC for CI Jakub Bartmiński
2018-07-12 14:26 ` [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests Chris Wilson
2018-07-12 14:49 ` Daniel Vetter
2018-07-12 14:48 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2018-10-16 9:42 [PATCH 1/2] drm/i915/guc: Limit number of scratch registers used for H2G Michal Wajdeczko
2018-10-16 9:42 ` [PATCH 2/2] HAX enable GuC for CI Michal Wajdeczko
2018-10-16 8:59 [PATCH 1/2] drm/i915/guc: Fix Gen9 GuC loading workarounds Michal Wajdeczko
2018-10-16 8:59 ` [PATCH 2/2] HAX enable GuC for CI Michal Wajdeczko
2018-10-15 22:10 [PATCH 1/2] drm/i915/guc: fix GuC suspend/resume Daniele Ceraolo Spurio
2018-10-15 22:10 ` [PATCH 2/2] HAX enable GuC for CI Daniele Ceraolo Spurio
2018-03-20 23:18 [PATCH 1/2] drm/i915: Add code to accept valid locked WOPCM register values Jackie Li
2018-03-20 23:18 ` [PATCH 2/2] HAX enable guc for CI Jackie Li
2018-02-23 14:10 [PATCH 1/2] drm/i915/uc: Introduce intel_uc_suspend|resume Michal Wajdeczko
2018-02-23 14:10 ` [PATCH 2/2] HAX: Enable GuC for CI Michal Wajdeczko
2018-02-23 9:04 [PATCH 1/2] drm/i915/preemption: Allow preemption between submission ports Chris Wilson
2018-02-23 9:04 ` [PATCH 2/2] HAX: enable GuC for CI Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox