Re: [Intel-gfx] [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
Quoting Ville Syrjälä (2018-03-02 08:27:45) > On Fri, Mar 02, 2018 at 08:12:35AM +, Chris Wilson wrote: > > Prevent the compiler from caching reads/writes to the hw register as we > > do want to perform mmio. > > > > Whilst fixing up the mmio access, also ensure that we do not leave the > > test with any other bits still set in the forcewake register to prevent > > affecting other tests, as spotted by Tvrtko. > > I wonder why this test isn't using intel_register_access_init() & co.? > Maybe because the library code used to take the forcewake always? But > that could be prevented with a setenv() now. It could just use intel_mmio_use_pci_bar() and then intel_register_access_init() doesn't have to worry about silly tests trying to abuse the hw. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
On Fri, Mar 02, 2018 at 08:12:35AM +, Chris Wilson wrote:
> Prevent the compiler from caching reads/writes to the hw register as we
> do want to perform mmio.
>
> Whilst fixing up the mmio access, also ensure that we do not leave the
> test with any other bits still set in the forcewake register to prevent
> affecting other tests, as spotted by Tvrtko.
I wonder why this test isn't using intel_register_access_init() & co.?
Maybe because the library code used to take the forcewake always? But
that could be prevented with a setenv() now.
>
> Signed-off-by: Chris Wilson
> Cc: Tvrtko Ursulin
> ---
> tests/gen7_forcewake_mt.c | 59
> +--
> 1 file changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
> index 07320ef9..218f674c 100644
> --- a/tests/gen7_forcewake_mt.c
> +++ b/tests/gen7_forcewake_mt.c
> @@ -44,9 +44,10 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround
> required for"
>
> struct thread {
> pthread_t thread;
> - void *mmio;
> + volatile uint32_t *mmio;
> int fd;
> int bit;
> + bool done;
> };
>
> static const struct pci_id_match match[] = {
> @@ -80,7 +81,7 @@ static struct pci_device *__igfx_get(void)
> return dev;
> }
>
> -static void *igfx_get_mmio(void)
> +static volatile uint32_t *igfx_get_forcewake_mt(void)
> {
> struct pci_device *pci = __igfx_get();
> void *mmio = NULL;
> @@ -100,20 +101,28 @@ static void *igfx_get_mmio(void)
> igt_assert_eq(error, 0);
> igt_assert(mmio != NULL);
>
> - return mmio;
> + return (volatile uint32_t *)((char *)mmio + FORCEWAKE_MT);
> }
>
> +
> static void *thread(void *arg)
> {
> + static const char acquire_error[] = "acquire";
> + static const char release_error[] = "release";
> +
> struct thread *t = arg;
> - uint32_t *forcewake_mt = (uint32_t *)((char *)t->mmio + FORCEWAKE_MT);
> - uint32_t bit = 1 << t->bit;
> + const uint32_t bit = 1 << t->bit;
> + volatile uint32_t *forcewake_mt = t->mmio;
>
> - while (1) {
> + while (!t->done) {
> *forcewake_mt = bit << 16 | bit;
> - igt_assert(*forcewake_mt & bit);
> + if (igt_wait(*forcewake_mt & bit, 50, 1))
> + return (void *)acquire_error;
> +
> *forcewake_mt = bit << 16;
> - igt_assert((*forcewake_mt & bit) == 0);
> + igt_assert(igt_wait((*forcewake_mt & bit) == 0, 50, 1) == 0);
> + if (igt_wait((*forcewake_mt & bit) == 0, 50, 1))
> + return (void *)release_error;
> }
>
> return NULL;
> @@ -124,10 +133,12 @@ static void *thread(void *arg)
> igt_simple_main
> {
> struct thread t[16];
> + bool success = true;
> int i;
>
> t[0].fd = drm_open_driver(DRIVER_INTEL);
> - t[0].mmio = igfx_get_mmio();
> + t[0].mmio = igfx_get_forcewake_mt();
> + t[0].done = false;
>
> for (i = 2; i < 16; i++) {
> t[i] = t[0];
> @@ -137,7 +148,7 @@ igt_simple_main
>
> sleep(2);
>
> - for (i = 0; i < 1000; i++) {
> + igt_until_timeout(2) {
> uint32_t *p;
> struct drm_i915_gem_execbuffer2 execbuf;
> struct drm_i915_gem_exec_object2 exec[2];
> @@ -192,13 +203,37 @@ igt_simple_main
> p = gem_mmap__gtt(t[0].fd, exec[0].handle, 4096, PROT_READ);
>
> igt_info("[%d]={ %08x %08x }\n", i, p[0], p[1]);
> - igt_assert(p[0] & 2);
> - igt_assert((p[1] & 2) == 0);
> + if ((p[0] & 2) == 0) {
> + igt_warn("Failed to acquire forcewake BIT(1) from
> batch\n");
> + success = false;
> + }
> + if ((p[1] & 2)) {
> + igt_warn("Failed to release forcewake BIT(1) from
> batch\n");
> + success = false;
> + }
>
> munmap(p, 4096);
> gem_close(t[0].fd, exec[0].handle);
> gem_close(t[0].fd, exec[1].handle);
> + if (!success)
> + break;
>
> usleep(1000);
> }
> +
> + for (i = 2; i < 16; i++) {
> + void *result;
> +
> + t[i].done = true;
> + pthread_join(t[i].thread, &result);
> + if (result) {
> + igt_warn("Thread BIT(%d) failed to %s forcewake\n", i,
> (char *)result);
> + success = false;
> + }
> + }
> +
> + /* And clear all forcewake bits before disappearing */
> + *t[0].mmio = 0xfffe << 16;
> +
> + igt_assert(success);
> }
> --
> 2.16.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_
[Intel-gfx] [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
Prevent the compiler from caching reads/writes to the hw register as we
do want to perform mmio.
Whilst fixing up the mmio access, also ensure that we do not leave the
test with any other bits still set in the forcewake register to prevent
affecting other tests, as spotted by Tvrtko.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
tests/gen7_forcewake_mt.c | 59 +--
1 file changed, 47 insertions(+), 12 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..218f674c 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -44,9 +44,10 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required
for"
struct thread {
pthread_t thread;
- void *mmio;
+ volatile uint32_t *mmio;
int fd;
int bit;
+ bool done;
};
static const struct pci_id_match match[] = {
@@ -80,7 +81,7 @@ static struct pci_device *__igfx_get(void)
return dev;
}
-static void *igfx_get_mmio(void)
+static volatile uint32_t *igfx_get_forcewake_mt(void)
{
struct pci_device *pci = __igfx_get();
void *mmio = NULL;
@@ -100,20 +101,28 @@ static void *igfx_get_mmio(void)
igt_assert_eq(error, 0);
igt_assert(mmio != NULL);
- return mmio;
+ return (volatile uint32_t *)((char *)mmio + FORCEWAKE_MT);
}
+
static void *thread(void *arg)
{
+ static const char acquire_error[] = "acquire";
+ static const char release_error[] = "release";
+
struct thread *t = arg;
- uint32_t *forcewake_mt = (uint32_t *)((char *)t->mmio + FORCEWAKE_MT);
- uint32_t bit = 1 << t->bit;
+ const uint32_t bit = 1 << t->bit;
+ volatile uint32_t *forcewake_mt = t->mmio;
- while (1) {
+ while (!t->done) {
*forcewake_mt = bit << 16 | bit;
- igt_assert(*forcewake_mt & bit);
+ if (igt_wait(*forcewake_mt & bit, 50, 1))
+ return (void *)acquire_error;
+
*forcewake_mt = bit << 16;
- igt_assert((*forcewake_mt & bit) == 0);
+ igt_assert(igt_wait((*forcewake_mt & bit) == 0, 50, 1) == 0);
+ if (igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +133,12 @@ static void *thread(void *arg)
igt_simple_main
{
struct thread t[16];
+ bool success = true;
int i;
t[0].fd = drm_open_driver(DRIVER_INTEL);
- t[0].mmio = igfx_get_mmio();
+ t[0].mmio = igfx_get_forcewake_mt();
+ t[0].done = false;
for (i = 2; i < 16; i++) {
t[i] = t[0];
@@ -137,7 +148,7 @@ igt_simple_main
sleep(2);
- for (i = 0; i < 1000; i++) {
+ igt_until_timeout(2) {
uint32_t *p;
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_exec_object2 exec[2];
@@ -192,13 +203,37 @@ igt_simple_main
p = gem_mmap__gtt(t[0].fd, exec[0].handle, 4096, PROT_READ);
igt_info("[%d]={ %08x %08x }\n", i, p[0], p[1]);
- igt_assert(p[0] & 2);
- igt_assert((p[1] & 2) == 0);
+ if ((p[0] & 2) == 0) {
+ igt_warn("Failed to acquire forcewake BIT(1) from
batch\n");
+ success = false;
+ }
+ if ((p[1] & 2)) {
+ igt_warn("Failed to release forcewake BIT(1) from
batch\n");
+ success = false;
+ }
munmap(p, 4096);
gem_close(t[0].fd, exec[0].handle);
gem_close(t[0].fd, exec[1].handle);
+ if (!success)
+ break;
usleep(1000);
}
+
+ for (i = 2; i < 16; i++) {
+ void *result;
+
+ t[i].done = true;
+ pthread_join(t[i].thread, &result);
+ if (result) {
+ igt_warn("Thread BIT(%d) failed to %s forcewake\n", i,
(char *)result);
+ success = false;
+ }
+ }
+
+ /* And clear all forcewake bits before disappearing */
+ *t[0].mmio = 0xfffe << 16;
+
+ igt_assert(success);
}
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
