* [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 8:12 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:12 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 8:12 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:12 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
@ 2018-03-02 8:18 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:18 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
One igt_assert() left inside the thread that was already duplicated into
the soft return.
---
tests/gen7_forcewake_mt.c | 58 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 12 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..49e92124 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,27 @@ 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);
+ if (igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +132,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 +147,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 +202,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 8:18 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:18 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
One igt_assert() left inside the thread that was already duplicated into
the soft return.
---
tests/gen7_forcewake_mt.c | 58 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 12 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..49e92124 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,27 @@ 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);
+ if (igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +132,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 +147,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 +202,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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH igt v4] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
@ 2018-03-02 8:22 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:22 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
Also be sure the compiler doesn't eliminate t->done.
Go get some coffee Chris.
-Chris
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..6eb66c00 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
" FORCEWAKE_MT.");
#define FORCEWAKE_MT 0xa188
+#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
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 +82,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 +102,27 @@ 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 (!READ_ONCE(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);
+ 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] [PATCH igt v4] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 8:22 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:22 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
Also be sure the compiler doesn't eliminate t->done.
Go get some coffee Chris.
-Chris
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..6eb66c00 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
" FORCEWAKE_MT.");
#define FORCEWAKE_MT 0xa188
+#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
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 +82,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 +102,27 @@ 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 (!READ_ONCE(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);
+ 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
@ 2018-03-02 8:27 ` Ville Syrjälä
-1 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2018-03-02 8:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
On Fri, Mar 02, 2018 at 08:12:35AM +0000, 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 <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> 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 mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Intel-gfx] [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 8:27 ` Ville Syrjälä
0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2018-03-02 8:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
On Fri, Mar 02, 2018 at 08:12:35AM +0000, 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 <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> 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 mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-02 8:27 ` [Intel-gfx] " Ville Syrjälä
@ 2018-03-02 8:35 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:35 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: igt-dev, intel-gfx
Quoting Ville Syrjälä (2018-03-02 08:27:45)
> On Fri, Mar 02, 2018 at 08:12:35AM +0000, 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 8:35 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 8:35 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: igt-dev, intel-gfx
Quoting Ville Syrjälä (2018-03-02 08:27:45)
> On Fri, Mar 02, 2018 at 08:12:35AM +0000, 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH igt v5] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
@ 2018-03-02 10:41 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 10:41 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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.
v2: Use intel_mmio_use_pci_bar() rather open code the ioremap
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tests/gen7_forcewake_mt.c | 76 +++++++++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 26 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..edf487a7 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
" FORCEWAKE_MT.");
#define FORCEWAKE_MT 0xa188
+#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
struct thread {
pthread_t thread;
- void *mmio;
+ volatile uint32_t *forcewake_mt;
int fd;
int bit;
+ bool done;
};
static const struct pci_id_match match[] = {
@@ -77,43 +79,39 @@ static struct pci_device *__igfx_get(void)
pci_iterator_destroy(iter);
}
+ pci_device_probe(dev);
return dev;
}
-static void *igfx_get_mmio(void)
+static volatile uint32_t *igfx_mmio_forcewake_mt(void)
{
struct pci_device *pci = __igfx_get();
- void *mmio = NULL;
- int error;
- igt_skip_on(pci == NULL);
- igt_skip_on(intel_gen(pci->device_id) != 7);
+ igt_require(pci);
+ igt_assert(intel_gen(pci->device_id) == 7);
- error = pci_device_probe(pci);
- igt_assert_eq(error, 0);
+ intel_mmio_use_pci_bar(pci);
- error = pci_device_map_range(pci,
- pci->regions[0].base_addr,
- 2*1024*1024,
- PCI_DEV_MAP_FLAG_WRITABLE,
- &mmio);
- igt_assert_eq(error, 0);
- igt_assert(mmio != NULL);
-
- return mmio;
+ return (volatile uint32_t *)((char *)igt_global_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->forcewake_mt;
- while (1) {
+ while (!READ_ONCE(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);
+ if (igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +122,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].forcewake_mt = igfx_mmio_forcewake_mt();
+ t[0].done = false;
for (i = 2; i < 16; i++) {
t[i] = t[0];
@@ -137,7 +137,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 +192,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].forcewake_mt = 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] [PATCH igt v5] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-02 10:41 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-02 10:41 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin
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.
v2: Use intel_mmio_use_pci_bar() rather open code the ioremap
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tests/gen7_forcewake_mt.c | 76 +++++++++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 26 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..edf487a7 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
" FORCEWAKE_MT.");
#define FORCEWAKE_MT 0xa188
+#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
struct thread {
pthread_t thread;
- void *mmio;
+ volatile uint32_t *forcewake_mt;
int fd;
int bit;
+ bool done;
};
static const struct pci_id_match match[] = {
@@ -77,43 +79,39 @@ static struct pci_device *__igfx_get(void)
pci_iterator_destroy(iter);
}
+ pci_device_probe(dev);
return dev;
}
-static void *igfx_get_mmio(void)
+static volatile uint32_t *igfx_mmio_forcewake_mt(void)
{
struct pci_device *pci = __igfx_get();
- void *mmio = NULL;
- int error;
- igt_skip_on(pci == NULL);
- igt_skip_on(intel_gen(pci->device_id) != 7);
+ igt_require(pci);
+ igt_assert(intel_gen(pci->device_id) == 7);
- error = pci_device_probe(pci);
- igt_assert_eq(error, 0);
+ intel_mmio_use_pci_bar(pci);
- error = pci_device_map_range(pci,
- pci->regions[0].base_addr,
- 2*1024*1024,
- PCI_DEV_MAP_FLAG_WRITABLE,
- &mmio);
- igt_assert_eq(error, 0);
- igt_assert(mmio != NULL);
-
- return mmio;
+ return (volatile uint32_t *)((char *)igt_global_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->forcewake_mt;
- while (1) {
+ while (!READ_ONCE(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);
+ if (igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +122,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].forcewake_mt = igfx_mmio_forcewake_mt();
+ t[0].done = false;
for (i = 2; i < 16; i++) {
t[i] = t[0];
@@ -137,7 +137,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 +192,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].forcewake_mt = 0xfffe << 16;
+
+ igt_assert(success);
}
--
2.16.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for igt/gen7_forcewake_mt: Make the mmio register as volatile (rev4)
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
` (4 preceding siblings ...)
(?)
@ 2018-03-05 13:56 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-03-05 13:56 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
== Series Details ==
Series: igt/gen7_forcewake_mt: Make the mmio register as volatile (rev4)
URL : https://patchwork.freedesktop.org/series/39255/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
23f7da18a92059610792299cfdb03d2c922a9948 lib/igt_pm: Restore runtime pm state on test exit
with latest DRM-Tip kernel build CI_DRM_3871
276a88800a08 drm-tip: 2018y-03m-05d-12h-15m-50s UTC integration manifest
No testlist changes.
---- Known issues:
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-cnl-y3) fdo#104951
incomplete -> PASS (fi-snb-2520m) fdo#103713
fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:425s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:424s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:374s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:487s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:282s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:482s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:490s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:468s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:462s
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:398s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:563s
fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:580s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:416s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:291s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:511s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:389s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:410s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:445s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:410s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:451s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:496s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:452s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:494s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:578s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:430s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:501s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:521s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:501s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:470s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:407s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:431s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:522s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:391s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1055/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for igt/gen7_forcewake_mt: Make the mmio register as volatile (rev4)
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
` (5 preceding siblings ...)
(?)
@ 2018-03-05 17:08 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-03-05 17:08 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
== Series Details ==
Series: igt/gen7_forcewake_mt: Make the mmio register as volatile (rev4)
URL : https://patchwork.freedesktop.org/series/39255/
State : failure
== Summary ==
---- Possible new issues:
Test drv_suspend:
Subgroup forcewake:
skip -> PASS (shard-snb)
Test gen7_forcewake_mt:
skip -> FAIL (shard-apl)
pass -> FAIL (shard-hsw)
skip -> FAIL (shard-snb)
---- Known issues:
Test gem_eio:
Subgroup in-flight-external:
incomplete -> PASS (shard-apl) fdo#104945 +1
Test kms_chv_cursor_fail:
Subgroup pipe-b-256x256-bottom-edge:
pass -> DMESG-WARN (shard-snb) fdo#105185 +1
Test kms_plane_multiple:
Subgroup atomic-pipe-b-tiling-yf:
fail -> PASS (shard-apl) fdo#103166
Test kms_rotation_crc:
Subgroup sprite-rotation-180:
pass -> FAIL (shard-snb) fdo#103925
Test kms_sysfs_edid_timing:
warn -> PASS (shard-apl) fdo#100047
Test perf:
Subgroup blocking:
fail -> PASS (shard-hsw) fdo#102252
fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-apl total:3398 pass:1790 dwarn:1 dfail:0 fail:8 skip:1598 time:12053s
shard-hsw total:3468 pass:1773 dwarn:1 dfail:0 fail:2 skip:1691 time:11869s
shard-snb total:3468 pass:1363 dwarn:2 dfail:0 fail:3 skip:2100 time:7194s
Blacklisted hosts:
shard-kbl total:3468 pass:1953 dwarn:1 dfail:0 fail:8 skip:1506 time:9437s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1055/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-02 10:41 ` [igt-dev] " Chris Wilson
@ 2018-03-05 17:30 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-05 17:30 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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.
v2: Use intel_mmio_use_pci_bar() rather open code the ioremap
v3: Flip igt_wait() checking as it returns true on success, not errno.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tests/gen7_forcewake_mt.c | 75 +++++++++++++++++++++++++++++++----------------
1 file changed, 49 insertions(+), 26 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..6818d7aa 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
" FORCEWAKE_MT.");
#define FORCEWAKE_MT 0xa188
+#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
struct thread {
pthread_t thread;
- void *mmio;
+ volatile uint32_t *forcewake_mt;
int fd;
int bit;
+ bool done;
};
static const struct pci_id_match match[] = {
@@ -77,43 +79,38 @@ static struct pci_device *__igfx_get(void)
pci_iterator_destroy(iter);
}
+ pci_device_probe(dev);
return dev;
}
-static void *igfx_get_mmio(void)
+static volatile uint32_t *igfx_mmio_forcewake_mt(void)
{
struct pci_device *pci = __igfx_get();
- void *mmio = NULL;
- int error;
- igt_skip_on(pci == NULL);
- igt_skip_on(intel_gen(pci->device_id) != 7);
+ igt_require(pci && intel_gen(pci->device_id) == 7);
- error = pci_device_probe(pci);
- igt_assert_eq(error, 0);
+ intel_mmio_use_pci_bar(pci);
- error = pci_device_map_range(pci,
- pci->regions[0].base_addr,
- 2*1024*1024,
- PCI_DEV_MAP_FLAG_WRITABLE,
- &mmio);
- igt_assert_eq(error, 0);
- igt_assert(mmio != NULL);
-
- return mmio;
+ return (volatile uint32_t *)((char *)igt_global_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->forcewake_mt;
- while (1) {
+ while (!READ_ONCE(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);
+ if (!igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +121,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].forcewake_mt = igfx_mmio_forcewake_mt();
+ t[0].done = false;
for (i = 2; i < 16; i++) {
t[i] = t[0];
@@ -137,7 +136,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 +191,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].forcewake_mt = 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Intel-gfx] [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-05 17:30 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-05 17:30 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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.
v2: Use intel_mmio_use_pci_bar() rather open code the ioremap
v3: Flip igt_wait() checking as it returns true on success, not errno.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tests/gen7_forcewake_mt.c | 75 +++++++++++++++++++++++++++++++----------------
1 file changed, 49 insertions(+), 26 deletions(-)
diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9..6818d7aa 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
" FORCEWAKE_MT.");
#define FORCEWAKE_MT 0xa188
+#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
struct thread {
pthread_t thread;
- void *mmio;
+ volatile uint32_t *forcewake_mt;
int fd;
int bit;
+ bool done;
};
static const struct pci_id_match match[] = {
@@ -77,43 +79,38 @@ static struct pci_device *__igfx_get(void)
pci_iterator_destroy(iter);
}
+ pci_device_probe(dev);
return dev;
}
-static void *igfx_get_mmio(void)
+static volatile uint32_t *igfx_mmio_forcewake_mt(void)
{
struct pci_device *pci = __igfx_get();
- void *mmio = NULL;
- int error;
- igt_skip_on(pci == NULL);
- igt_skip_on(intel_gen(pci->device_id) != 7);
+ igt_require(pci && intel_gen(pci->device_id) == 7);
- error = pci_device_probe(pci);
- igt_assert_eq(error, 0);
+ intel_mmio_use_pci_bar(pci);
- error = pci_device_map_range(pci,
- pci->regions[0].base_addr,
- 2*1024*1024,
- PCI_DEV_MAP_FLAG_WRITABLE,
- &mmio);
- igt_assert_eq(error, 0);
- igt_assert(mmio != NULL);
-
- return mmio;
+ return (volatile uint32_t *)((char *)igt_global_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->forcewake_mt;
- while (1) {
+ while (!READ_ONCE(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);
+ if (!igt_wait((*forcewake_mt & bit) == 0, 50, 1))
+ return (void *)release_error;
}
return NULL;
@@ -124,10 +121,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].forcewake_mt = igfx_mmio_forcewake_mt();
+ t[0].done = false;
for (i = 2; i < 16; i++) {
t[i] = t[0];
@@ -137,7 +136,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 +191,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].forcewake_mt = 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for igt/gen7_forcewake_mt: Make the mmio register as volatile (rev5)
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
` (6 preceding siblings ...)
(?)
@ 2018-03-05 18:24 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-03-05 18:24 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
== Series Details ==
Series: igt/gen7_forcewake_mt: Make the mmio register as volatile (rev5)
URL : https://patchwork.freedesktop.org/series/39255/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
23f7da18a92059610792299cfdb03d2c922a9948 lib/igt_pm: Restore runtime pm state on test exit
with latest DRM-Tip kernel build CI_DRM_3872
4a3784737699 drm-tip: 2018y-03m-05d-14h-32m-57s UTC integration manifest
No testlist changes.
---- Known issues:
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test prime_vgem:
Subgroup basic-fence-flip:
fail -> PASS (fi-ilk-650) fdo#104008
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
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:421s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:425s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:374s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:490s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:278s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:488s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:470s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:460s
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:391s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:563s
fi-cfl-u total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:498s
fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:570s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:415s
fi-gdg-551 total:288 pass:180 dwarn:0 dfail:0 fail:0 skip:108 time:288s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:506s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:388s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:415s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:454s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:413s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:460s
fi-kbl-7560u total:108 pass:104 dwarn:0 dfail:0 fail:0 skip:3
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:448s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:493s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:583s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:425s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:503s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:518s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:501s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:475s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:405s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:430s
fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:393s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1062/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: warning for igt/gen7_forcewake_mt: Make the mmio register as volatile (rev5)
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
` (7 preceding siblings ...)
(?)
@ 2018-03-06 0:09 ` Patchwork
-1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-03-06 0:09 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
== Series Details ==
Series: igt/gen7_forcewake_mt: Make the mmio register as volatile (rev5)
URL : https://patchwork.freedesktop.org/series/39255/
State : warning
== Summary ==
---- Possible new issues:
Test kms_concurrent:
Subgroup pipe-a:
pass -> DMESG-WARN (shard-apl)
Test kms_vblank:
Subgroup pipe-a-ts-continuation-modeset:
skip -> PASS (shard-snb)
---- Known issues:
Test gem_eio:
Subgroup in-flight-contexts:
pass -> INCOMPLETE (shard-apl) fdo#104945
Test kms_chv_cursor_fail:
Subgroup pipe-b-256x256-top-edge:
pass -> DMESG-WARN (shard-snb) fdo#105185 +2
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-atomic:
skip -> PASS (shard-snb) fdo#102670
Test kms_flip:
Subgroup 2x-plain-flip-ts-check:
pass -> FAIL (shard-hsw) fdo#100368
Subgroup dpms-vs-vblank-race:
fail -> PASS (shard-hsw) fdo#103060
Test kms_sysfs_edid_timing:
warn -> PASS (shard-apl) fdo#100047
fdo#104945 https://bugs.freedesktop.org/show_bug.cgi?id=104945
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
shard-apl total:3434 pass:1805 dwarn:2 dfail:0 fail:7 skip:1619 time:11828s
shard-hsw total:3468 pass:1773 dwarn:1 dfail:0 fail:2 skip:1691 time:11723s
shard-snb total:3468 pass:1363 dwarn:3 dfail:0 fail:1 skip:2101 time:7070s
Blacklisted hosts:
shard-kbl total:3434 pass:1930 dwarn:1 dfail:0 fail:7 skip:1495 time:9156s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1062/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-05 17:30 ` [Intel-gfx] " Chris Wilson
@ 2018-03-06 9:37 ` Tvrtko Ursulin
-1 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2018-03-06 9:37 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
On 05/03/2018 17:30, 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.
>
> v2: Use intel_mmio_use_pci_bar() rather open code the ioremap
> v3: Flip igt_wait() checking as it returns true on success, not errno.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> tests/gen7_forcewake_mt.c | 75 +++++++++++++++++++++++++++++++----------------
> 1 file changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
> index 07320ef9..6818d7aa 100644
> --- a/tests/gen7_forcewake_mt.c
> +++ b/tests/gen7_forcewake_mt.c
> @@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
> " FORCEWAKE_MT.");
>
> #define FORCEWAKE_MT 0xa188
> +#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
>
> struct thread {
> pthread_t thread;
> - void *mmio;
> + volatile uint32_t *forcewake_mt;
> int fd;
> int bit;
> + bool done;
> };
>
> static const struct pci_id_match match[] = {
> @@ -77,43 +79,38 @@ static struct pci_device *__igfx_get(void)
> pci_iterator_destroy(iter);
> }
>
> + pci_device_probe(dev);
> return dev;
> }
>
> -static void *igfx_get_mmio(void)
> +static volatile uint32_t *igfx_mmio_forcewake_mt(void)
I think its overkill to bother with volatile all throughout the chain,
it's not like it's const. But never mind.
> {
> struct pci_device *pci = __igfx_get();
> - void *mmio = NULL;
> - int error;
>
> - igt_skip_on(pci == NULL);
> - igt_skip_on(intel_gen(pci->device_id) != 7);
> + igt_require(pci && intel_gen(pci->device_id) == 7);
>
> - error = pci_device_probe(pci);
> - igt_assert_eq(error, 0);
> + intel_mmio_use_pci_bar(pci);
>
> - error = pci_device_map_range(pci,
> - pci->regions[0].base_addr,
> - 2*1024*1024,
> - PCI_DEV_MAP_FLAG_WRITABLE,
> - &mmio);
> - igt_assert_eq(error, 0);
> - igt_assert(mmio != NULL);
> -
> - return mmio;
> + return (volatile uint32_t *)((char *)igt_global_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->forcewake_mt;
>
> - while (1) {
> + while (!READ_ONCE(t->done)) {
Not really important, but isn't this not allowed to be compiled out
since it comes from external to the function memory?
> *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);
> + if (!igt_wait((*forcewake_mt & bit) == 0, 50, 1))
> + return (void *)release_error;
> }
>
> return NULL;
> @@ -124,10 +121,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].forcewake_mt = igfx_mmio_forcewake_mt();
> + t[0].done = false;
>
> for (i = 2; i < 16; i++) {
> t[i] = t[0];
> @@ -137,7 +136,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 +191,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);
Check return value just in case?
> + 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].forcewake_mt = 0xfffe << 16;
> +
> + igt_assert(success);
> }
>
But only minor comments so:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-06 9:37 ` Tvrtko Ursulin
0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2018-03-06 9:37 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
On 05/03/2018 17:30, 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.
>
> v2: Use intel_mmio_use_pci_bar() rather open code the ioremap
> v3: Flip igt_wait() checking as it returns true on success, not errno.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> tests/gen7_forcewake_mt.c | 75 +++++++++++++++++++++++++++++++----------------
> 1 file changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
> index 07320ef9..6818d7aa 100644
> --- a/tests/gen7_forcewake_mt.c
> +++ b/tests/gen7_forcewake_mt.c
> @@ -41,12 +41,14 @@ IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
> " FORCEWAKE_MT.");
>
> #define FORCEWAKE_MT 0xa188
> +#define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
>
> struct thread {
> pthread_t thread;
> - void *mmio;
> + volatile uint32_t *forcewake_mt;
> int fd;
> int bit;
> + bool done;
> };
>
> static const struct pci_id_match match[] = {
> @@ -77,43 +79,38 @@ static struct pci_device *__igfx_get(void)
> pci_iterator_destroy(iter);
> }
>
> + pci_device_probe(dev);
> return dev;
> }
>
> -static void *igfx_get_mmio(void)
> +static volatile uint32_t *igfx_mmio_forcewake_mt(void)
I think its overkill to bother with volatile all throughout the chain,
it's not like it's const. But never mind.
> {
> struct pci_device *pci = __igfx_get();
> - void *mmio = NULL;
> - int error;
>
> - igt_skip_on(pci == NULL);
> - igt_skip_on(intel_gen(pci->device_id) != 7);
> + igt_require(pci && intel_gen(pci->device_id) == 7);
>
> - error = pci_device_probe(pci);
> - igt_assert_eq(error, 0);
> + intel_mmio_use_pci_bar(pci);
>
> - error = pci_device_map_range(pci,
> - pci->regions[0].base_addr,
> - 2*1024*1024,
> - PCI_DEV_MAP_FLAG_WRITABLE,
> - &mmio);
> - igt_assert_eq(error, 0);
> - igt_assert(mmio != NULL);
> -
> - return mmio;
> + return (volatile uint32_t *)((char *)igt_global_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->forcewake_mt;
>
> - while (1) {
> + while (!READ_ONCE(t->done)) {
Not really important, but isn't this not allowed to be compiled out
since it comes from external to the function memory?
> *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);
> + if (!igt_wait((*forcewake_mt & bit) == 0, 50, 1))
> + return (void *)release_error;
> }
>
> return NULL;
> @@ -124,10 +121,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].forcewake_mt = igfx_mmio_forcewake_mt();
> + t[0].done = false;
>
> for (i = 2; i < 16; i++) {
> t[i] = t[0];
> @@ -137,7 +136,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 +191,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);
Check return value just in case?
> + 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].forcewake_mt = 0xfffe << 16;
> +
> + igt_assert(success);
> }
>
But only minor comments so:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
2018-03-06 9:37 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2018-03-06 9:44 ` Chris Wilson
-1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-06 9:44 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev
Quoting Tvrtko Ursulin (2018-03-06 09:37:50)
>
> On 05/03/2018 17:30, Chris Wilson wrote:
> > -static void *igfx_get_mmio(void)
> > +static volatile uint32_t *igfx_mmio_forcewake_mt(void)
>
> I think its overkill to bother with volatile all throughout the chain,
> it's not like it's const. But never mind.
When you stub your toe, you should amputate the leg. Just to be sure.
> > 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->forcewake_mt;
> >
> > - while (1) {
> > + while (!READ_ONCE(t->done)) {
>
> Not really important, but isn't this not allowed to be compiled out
> since it comes from external to the function memory?
If there wasn't an ::memory barrier inside the loop, the compiler can
assume that the value doesn't change. (It is not marked as volatile).
However, we do have volatiles (and so external memory barriers) inside
the loop, so should be a moot point.
> > *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);
> > + if (!igt_wait((*forcewake_mt & bit) == 0, 50, 1))
> > + return (void *)release_error;
> > }
> > +
> > + for (i = 2; i < 16; i++) {
> > + void *result;
> > +
> > + t[i].done = true;
> > + pthread_join(t[i].thread, &result);
>
> Check return value just in case?
Here it can only fail in a programming error, shrug. We are better off
with an igt_assert() on pthread_create().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH igt v3] igt/gen7_forcewake_mt: Make the mmio register as volatile
@ 2018-03-06 9:44 ` Chris Wilson
0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-03-06 9:44 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev
Quoting Tvrtko Ursulin (2018-03-06 09:37:50)
>
> On 05/03/2018 17:30, Chris Wilson wrote:
> > -static void *igfx_get_mmio(void)
> > +static volatile uint32_t *igfx_mmio_forcewake_mt(void)
>
> I think its overkill to bother with volatile all throughout the chain,
> it's not like it's const. But never mind.
When you stub your toe, you should amputate the leg. Just to be sure.
> > 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->forcewake_mt;
> >
> > - while (1) {
> > + while (!READ_ONCE(t->done)) {
>
> Not really important, but isn't this not allowed to be compiled out
> since it comes from external to the function memory?
If there wasn't an ::memory barrier inside the loop, the compiler can
assume that the value doesn't change. (It is not marked as volatile).
However, we do have volatiles (and so external memory barriers) inside
the loop, so should be a moot point.
> > *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);
> > + if (!igt_wait((*forcewake_mt & bit) == 0, 50, 1))
> > + return (void *)release_error;
> > }
> > +
> > + for (i = 2; i < 16; i++) {
> > + void *result;
> > +
> > + t[i].done = true;
> > + pthread_join(t[i].thread, &result);
>
> Check return value just in case?
Here it can only fail in a programming error, shrug. We are better off
with an igt_assert() on pthread_create().
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-03-06 9:44 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 8:12 [PATCH igt v2] igt/gen7_forcewake_mt: Make the mmio register as volatile Chris Wilson
2018-03-02 8:12 ` [igt-dev] " Chris Wilson
2018-03-02 8:18 ` [PATCH igt v3] " Chris Wilson
2018-03-02 8:18 ` [igt-dev] " Chris Wilson
2018-03-02 8:22 ` [PATCH igt v4] " Chris Wilson
2018-03-02 8:22 ` [igt-dev] " Chris Wilson
2018-03-02 8:27 ` [PATCH igt v2] " Ville Syrjälä
2018-03-02 8:27 ` [Intel-gfx] " Ville Syrjälä
2018-03-02 8:35 ` Chris Wilson
2018-03-02 8:35 ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-03-02 10:41 ` [PATCH igt v5] " Chris Wilson
2018-03-02 10:41 ` [igt-dev] " Chris Wilson
2018-03-05 17:30 ` [PATCH igt v3] " Chris Wilson
2018-03-05 17:30 ` [Intel-gfx] " Chris Wilson
2018-03-06 9:37 ` Tvrtko Ursulin
2018-03-06 9:37 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2018-03-06 9:44 ` Chris Wilson
2018-03-06 9:44 ` [igt-dev] [Intel-gfx] " Chris Wilson
2018-03-05 13:56 ` [igt-dev] ✓ Fi.CI.BAT: success for igt/gen7_forcewake_mt: Make the mmio register as volatile (rev4) Patchwork
2018-03-05 17:08 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2018-03-05 18:24 ` [igt-dev] ✓ Fi.CI.BAT: success for igt/gen7_forcewake_mt: Make the mmio register as volatile (rev5) Patchwork
2018-03-06 0:09 ` [igt-dev] ✗ Fi.CI.IGT: warning " Patchwork
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.