[PATCH i-g-t] tests/gem_workarounds: reduce scope of intel_register_access_init

[PATCH i-g-t] tests/gem_workarounds: reduce scope of intel_register_access_init

[Matthew Auld]

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(-)

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();
 	}
 
 }
-- 
2.4.11

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

Re: [PATCH i-g-t] tests/gem_workarounds: reduce scope of intel_register_access_init

[Chris Wilson]

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

Re: [PATCH i-g-t] tests/gem_workarounds: reduce scope of intel_register_access_init

[Matthew Auld]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] tests/gem_workarounds: reduce scope of intel_register_access_init

[Chris Wilson]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] tests/gem_workarounds: reduce scope of intel_register_access_init

[Matthew Auld]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx