| Message ID | 1465482827-14883-1-git-send-email-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 09, 2016 at 03:33:47PM +0100, Matthew Auld wrote: > We shouldn't be holding the forcewake whilst going through suspend-resume > cycle Why not? That's legal behaviour, the kernel should be restoring the user forcewake setting and there should be igt for that already - otherwise we have some rather scary code in intel_uncore.c that isn't being exercised. -Chris
Well, at least for me having the global forcewake produces a kernel warning during the suspend-resume test case. On 9 June 2016 at 15:53, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, Jun 09, 2016 at 03:33:47PM +0100, Matthew Auld wrote: >> We shouldn't be holding the forcewake whilst going through suspend-resume >> cycle > > Why not? That's legal behaviour, the kernel should be restoring the user > forcewake setting and there should be igt for that already - otherwise > we have some rather scary code in intel_uncore.c that isn't being > exercised. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 09, 2016 at 04:02:26PM +0100, Matthew Auld wrote: > Well, at least for me having the global forcewake produces a kernel > warning during the suspend-resume test case. If userspace can trigger a kernel warning, we fix the kernel... Except this is debugfs, and if necessary we can caveat by saying don't do that if necessary. On resume, we take action to try and restore the userspace forcewake count, so what's the exact warning? -Chris
https://paste.fedoraproject.org/376691/ On 9 June 2016 at 16:22, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, Jun 09, 2016 at 04:02:26PM +0100, Matthew Auld wrote: >> Well, at least for me having the global forcewake produces a kernel >> warning during the suspend-resume test case. > > If userspace can trigger a kernel warning, we fix the kernel... > > Except this is debugfs, and if necessary we can caveat by saying don't > do that if necessary. On resume, we take action to try and restore the > userspace forcewake count, so what's the exact warning? > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c index e419e8b..74799fd 100644 --- a/tests/gem_workarounds.c +++ b/tests/gem_workarounds.c @@ -67,6 +67,8 @@ static int workaround_fail_count(void) { int i, fail_count = 0; + intel_register_access_init(intel_get_pci_device(), 0); + /* There is a small delay after coming ot of rc6 to the correct render context values will get loaded by hardware (bdw,chv). This here ensures that we have the correct context loaded before @@ -93,6 +95,8 @@ static int workaround_fail_count(void) } } + intel_register_access_fini(); + return fail_count; } @@ -131,8 +135,6 @@ igt_main pci_dev = intel_get_pci_device(); igt_require(pci_dev); - intel_register_access_init(pci_dev, 0); - file = igt_debugfs_fopen("i915_wa_registers", "r"); igt_assert(getline(&line, &line_size, file) > 0); igt_debug("i915_wa_registers: %s", line); @@ -173,7 +175,6 @@ igt_main igt_fixture { free(wa_regs); - intel_register_access_fini(); } }
We shouldn't be holding the forcewake whilst going through suspend-resume cycle, so instead of globally holding the forcewake we reduce this to when we actually need to read the registers. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Matthew Auld <matthew.auld@intel.com> --- tests/gem_workarounds.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)