Re: [Intel-gfx] [PATCH 12/20] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-17 Thread Sagar Arun Kamble



On 2/17/2018 5:48 PM, Chris Wilson wrote:

Quoting Sagar Arun Kamble (2018-02-17 12:10:32)


On 2/17/2018 2:34 PM, Chris Wilson wrote:

Quoting Sagar Arun Kamble (2018-02-17 08:51:44)

Earlier I had thought of calling ASSIGN_FW_DOMAINS_TABLE,
ASSIGN_*_MMIO_VFUNCS before intel_uncore_fw_domains_init
and use I915_READ here for reading the fuse. But that approach seems to
expose the vfuncs and forcewake table before fw domains
are initialized. Although we can get to know invalid access to
read/write accessors before fw_domains get initialized, current ordering
of fw_domains init followed by fw domain table/read/write vfuncs init
seems right. So using fw_domains_get/put as suggested above
should be the way.

Chris, Tvrtko, Mika, do you agree?

What's the complication with the fw_domains? Do the additional powerwell
depend on the fused status of the extra engines? Does it matter if the
fw_domain are prepped if they are never used?

Yes. To discover the available VD/VE engines/power domains, fuse needs
to be read under blitter forcewake as
RC6 will be enabled by BIOS. We do have usage of forcewake in IVB to
discover FORCEWAKE_MT availability in fw_domains_init.
It should not be a problem if they are prepped but never used.

Imo, I would have placed the fused discovery in
intel_engines_init_mmio() (where we do the setup and can take forcewake).
Then adding something like intel_uncore_reinit_mmio() (which would just
prune the uncore->fw_domains) after checking fused status with commentary
doesn't seem that horrible.

Yes. This approach looks good too. But, we might want to optimize the
driver_load to avoid this setup at first place
instead of pruning later.

How many cycles does it take to run through all domains and set up the
register offsets? No mmio access required right, we are just moving
memory around without even hitting locked instructions?
Yes. Latency is minimal. We can go with your suggestion. Will need to 
maintain separation of engine_cs/device_info/uncore update

through separate functions.

Another related setup that can be avoided/pruned is the fw_range for the
engine fused off as it can improve the fw lookup.

Which is a bsearch on register range, I doubt that's going to be
substantially impacted by removing a few ranges. Where it matters, we
should be looking to precalculate the result anyway.

Yes. seems not so worth.

-Chris


--
Thanks,
Sagar

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


Re: [Intel-gfx] [PATCH 12/20] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-17 Thread Chris Wilson
Quoting Sagar Arun Kamble (2018-02-17 12:10:32)
> 
> 
> On 2/17/2018 2:34 PM, Chris Wilson wrote:
> > Quoting Sagar Arun Kamble (2018-02-17 08:51:44)
> >> Earlier I had thought of calling ASSIGN_FW_DOMAINS_TABLE,
> >> ASSIGN_*_MMIO_VFUNCS before intel_uncore_fw_domains_init
> >> and use I915_READ here for reading the fuse. But that approach seems to
> >> expose the vfuncs and forcewake table before fw domains
> >> are initialized. Although we can get to know invalid access to
> >> read/write accessors before fw_domains get initialized, current ordering
> >> of fw_domains init followed by fw domain table/read/write vfuncs init
> >> seems right. So using fw_domains_get/put as suggested above
> >> should be the way.
> >>
> >> Chris, Tvrtko, Mika, do you agree?
> > What's the complication with the fw_domains? Do the additional powerwell
> > depend on the fused status of the extra engines? Does it matter if the
> > fw_domain are prepped if they are never used?
> Yes. To discover the available VD/VE engines/power domains, fuse needs 
> to be read under blitter forcewake as
> RC6 will be enabled by BIOS. We do have usage of forcewake in IVB to 
> discover FORCEWAKE_MT availability in fw_domains_init.
> It should not be a problem if they are prepped but never used.
> > Imo, I would have placed the fused discovery in
> > intel_engines_init_mmio() (where we do the setup and can take forcewake).
> > Then adding something like intel_uncore_reinit_mmio() (which would just
> > prune the uncore->fw_domains) after checking fused status with commentary
> > doesn't seem that horrible.
> Yes. This approach looks good too. But, we might want to optimize the 
> driver_load to avoid this setup at first place
> instead of pruning later.

How many cycles does it take to run through all domains and set up the
register offsets? No mmio access required right, we are just moving
memory around without even hitting locked instructions?

> Another related setup that can be avoided/pruned is the fw_range for the 
> engine fused off as it can improve the fw lookup.

Which is a bsearch on register range, I doubt that's going to be
substantially impacted by removing a few ranges. Where it matters, we
should be looking to precalculate the result anyway.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 12/20] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-17 Thread Sagar Arun Kamble



On 2/17/2018 2:34 PM, Chris Wilson wrote:

Quoting Sagar Arun Kamble (2018-02-17 08:51:44)

Earlier I had thought of calling ASSIGN_FW_DOMAINS_TABLE,
ASSIGN_*_MMIO_VFUNCS before intel_uncore_fw_domains_init
and use I915_READ here for reading the fuse. But that approach seems to
expose the vfuncs and forcewake table before fw domains
are initialized. Although we can get to know invalid access to
read/write accessors before fw_domains get initialized, current ordering
of fw_domains init followed by fw domain table/read/write vfuncs init
seems right. So using fw_domains_get/put as suggested above
should be the way.

Chris, Tvrtko, Mika, do you agree?

What's the complication with the fw_domains? Do the additional powerwell
depend on the fused status of the extra engines? Does it matter if the
fw_domain are prepped if they are never used?
Yes. To discover the available VD/VE engines/power domains, fuse needs 
to be read under blitter forcewake as
RC6 will be enabled by BIOS. We do have usage of forcewake in IVB to 
discover FORCEWAKE_MT availability in fw_domains_init.

It should not be a problem if they are prepped but never used.

Imo, I would have placed the fused discovery in
intel_engines_init_mmio() (where we do the setup and can take forcewake).
Then adding something like intel_uncore_reinit_mmio() (which would just
prune the uncore->fw_domains) after checking fused status with commentary
doesn't seem that horrible.
Yes. This approach looks good too. But, we might want to optimize the 
driver_load to avoid this setup at first place

instead of pruning later.
Another related setup that can be avoided/pruned is the fw_range for the 
engine fused off as it can improve the fw lookup.

-Chris


--
Thanks,
Sagar

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


[Intel-gfx] [PATCH igt 2/2] tests/gem_ctx_param: Update invalid param

2018-02-17 Thread Chris Wilson
From: Antonio Argenziano 

Since commit: drm/i915/scheduler: Support user-defined priorities, the
driver support an extra context param to set context's priority. Add
tests for that interface and update invalid tests.

v2:
- Add arg size validation test. (Chris)
- Add arg value overflow test. (Chris)
- Add test for unsupported platforms. (Chris)
- Feed interface with all priority values and in random order. (Chris)

v3:
- Parametrize tests. (Chris)

v4:
- Code-style refactoring. (Chris)

Signed-off-by: Antonio Argenziano 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Cc: Tvrtko Ursulin 
Reviewed-by: Chris Wilson 
---
 tests/gem_ctx_param.c | 158 +-
 1 file changed, 157 insertions(+), 1 deletion(-)

diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index db68f57b..c46fd709 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -24,10 +24,125 @@
  *Daniel Vetter 
  */
 
+#include 
+#include 
+
 #include "igt.h"
 
 IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 
+#define BIT(x) (1ul << (x))
+
+#define NEW_CTXBIT(0)
+#define USER BIT(1)
+
+static int reopen_driver(int fd)
+{
+   char path[256];
+
+   snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+   fd = open(path, O_RDWR);
+   igt_assert_lte(0, fd);
+
+   return fd;
+}
+
+static void set_priority(int i915)
+{
+   static const int64_t test_values[] = {
+   /* Test space too big, pick significant values */
+   INT_MIN,
+
+   I915_CONTEXT_MIN_USER_PRIORITY - 1,
+   I915_CONTEXT_MIN_USER_PRIORITY,
+   I915_CONTEXT_MIN_USER_PRIORITY + 1,
+
+   I915_CONTEXT_DEFAULT_PRIORITY - 1,
+   I915_CONTEXT_DEFAULT_PRIORITY,
+   I915_CONTEXT_DEFAULT_PRIORITY + 1,
+
+   I915_CONTEXT_MAX_USER_PRIORITY - 1,
+   I915_CONTEXT_MAX_USER_PRIORITY,
+   I915_CONTEXT_MAX_USER_PRIORITY + 1,
+
+   INT_MAX
+   };
+   unsigned int size;
+   int64_t *values;
+
+   igt_require(getuid() == 0);
+
+   size = ARRAY_SIZE(test_values);
+   values = malloc(sizeof(test_values) * 8);
+   igt_assert(values);
+
+   for (unsigned i = 0; i < size; i++) {
+   values[i + 0*size] = test_values[i];
+   values[i + 1*size] = test_values[i] | (uint64_t)1 << 32;
+   values[i + 2*size] = test_values[i] | (uint64_t)rand() << 32;
+   values[i + 3*size] = test_values[i] ^ rand();
+   values[i + 4*size] = rand() % (I915_CONTEXT_MAX_USER_PRIORITY - 
I915_CONTEXT_MIN_USER_PRIORITY) + I915_CONTEXT_MIN_USER_PRIORITY;
+   values[i + 5*size] = rand();
+   values[i + 6*size] = rand() | (uint64_t)rand() << 32;
+   values[i + 7*size] = (uint64_t)test_values[i] << 32;
+   }
+   size *= 8;
+
+   igt_permute_array(values, size, igt_exchange_int64);
+
+   igt_fork(flags, NEW_CTX | USER) {
+   int fd = reopen_driver(i915);
+   struct drm_i915_gem_context_param arg = {
+   .param = I915_CONTEXT_PARAM_PRIORITY,
+   .ctx_id = flags & NEW_CTX ? gem_context_create(fd) : 0,
+   };
+   int64_t old_prio;
+
+   if (flags & USER) {
+   igt_debug("Dropping root privilege\n");
+   igt_drop_root();
+   }
+
+   gem_context_get_param(fd, );
+   old_prio = arg.value;
+
+   for (unsigned i = 0; i < size; i++) {
+   int64_t prio = values[i];
+   int expected = 0;
+   int err;
+
+   arg.value = prio;
+
+   if (flags & USER &&
+   prio > I915_CONTEXT_DEFAULT_PRIORITY)
+   expected = -EPERM;
+
+   if (prio < I915_CONTEXT_MIN_USER_PRIORITY ||
+   prio > I915_CONTEXT_MAX_USER_PRIORITY)
+   expected = -EINVAL;
+
+   err =__gem_context_set_param(fd, );
+   igt_assert_f(err == expected,
+"Priority requested %" PRId64 " with flags 
%x, expected result %d, returned %d\n",
+prio, flags, expected, err);
+
+   gem_context_get_param(fd, );
+   if (!err)
+   old_prio = prio;
+   igt_assert_eq(arg.value, 

[Intel-gfx] [PATCH igt 1/2] lib/igt_aux: Add function to swap int64 in array

2018-02-17 Thread Chris Wilson
From: Antonio Argenziano 

v2: Use igt_swap()

Signed-off-by: Antonio Argenziano 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Reviewed-by: Chris Wilson 
---
 lib/igt_aux.c | 16 
 lib/igt_aux.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 8ca0b60d..a23375a4 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -577,6 +577,22 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
int_arr[j] = tmp;
 }
 
+/**
+ * igt_exchange_int64:
+ * @array: pointer to the array of int64_t
+ * @i: first position
+ * @j: second position
+ *
+ * Exchanges the two values at array indices @i and @j. Useful as an exchange
+ * function for igt_permute_array().
+ */
+void igt_exchange_int64(void *array, unsigned i, unsigned j)
+{
+   int64_t *a = array;
+
+   igt_swap(a[i], a[j]);
+}
+
 /**
  * igt_permute_array:
  * @array: pointer to array
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index f9c75992..43dd15fe 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -118,6 +118,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, 
bool interrupt);
for (struct timespec t__={}; igt_nsec_elapsed(__)>>20 < (t); )
 
 void igt_exchange_int(void *array, unsigned i, unsigned j);
+void igt_exchange_int64(void *array, unsigned i, unsigned j);
 void igt_permute_array(void *array, unsigned size,
   void (*exchange_func)(void *array,
 unsigned i,
-- 
2.16.1

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


Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v6] tests/perf_pmu: Verify engine busyness accuracy

2018-02-17 Thread Chris Wilson
Quoting Tvrtko Ursulin (2018-02-15 15:34:53)
> From: Tvrtko Ursulin 
> 
> A subtest to verify that the engine busyness is reported with expected
> accuracy on platforms where the feature is available.
> 
> We test three patterns: 2%, 50% and 98% load per engine.
> 
> v2:
>  * Use spin batch instead of nop calibration.
>  * Various tweaks.
> 
> v3:
>  * Change loops to be time based.
>  * Use __igt_spin_batch_new inside timing sensitive loops.
>  * Fixed PWM sleep handling.
> 
> v4:
>  * Use restarting spin batch.
>  * Calibrate more carefully by looking at the real PWM loop.
> 
> v5:
>  * Made standalone.
>  * Better info messages.
>  * Tweak sleep compensation.
> 
> v6:
>  * Some final tweaks. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin 
> Reviewed-by: Chris Wilson 
> ---
> +
> +   /* Sampling platforms cannot reach the high accuracy criteria. */
> +   igt_require(gem_has_execlists(gem_fd));

But we don't handle guc, right?
igt_skip_on(gem_has_guc_submission(gem_fd)) ?

https://intel-gfx-ci.01.org/tree/drm-tip/kasan_2/fi-skl-guc/igt@perf_...@busy-accuracy-2-vecs0.html

Or at least it doesn't work to sufficient accuracy. And bsw hung.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH igt] lib: Remove overzealous assertion on gem_set_caching()

2018-02-17 Thread Chris Wilson
When the asserts were added for the acceptable error codes for
SET_CACHING ioctl, foresight was not given to the possibility that the
device may not handle the caching mode and return -ENODEV. Remove the
error code assertion from the library, that is the job for the ABI
tests.

Signed-off-by: Chris Wilson 
---
 lib/ioctl_wrappers.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 7b656cce..8748cfcf 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -208,10 +208,10 @@ static int __gem_set_caching(int fd, uint32_t handle, 
uint32_t caching)
arg.caching = caching;
 
err = 0;
-   if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_SET_CACHING, )) {
+   if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_SET_CACHING, ))
err = -errno;
-   igt_assert(errno == ENOTTY || errno == EINVAL);
-   }
+
+   errno = 0;
return err;
 }
 
@@ -229,7 +229,6 @@ static int __gem_set_caching(int fd, uint32_t handle, 
uint32_t caching)
 void gem_set_caching(int fd, uint32_t handle, uint32_t caching)
 {
igt_require(__gem_set_caching(fd, handle, caching) == 0);
-   errno = 0;
 }
 
 /**
-- 
2.16.1

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


Re: [Intel-gfx] [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers

2018-02-17 Thread Lukas Wunner
On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote:
>   workqueue: Allow retrieval of current task's work struct
>   drm: Allow determining if current task is output poll worker
>   drm/nouveau: Fix deadlock on runtime suspend
>   drm/radeon: Fix deadlock on runtime suspend
>   drm/amdgpu: Fix deadlock on runtime suspend

Pushed to drm-misc-fixes, thanks a lot everyone for the acks,
reviews, testing and comments.

drm-misc maintainers, heads-up:

drm-misc-fixes is still based on 4.15-rc8.  The present series
applies cleanly to both 4.15 and 4.16, so I had no need to have
4.16-rc1 backmerged, but that may be necessary sooner or later.
I did a local test pull into drm-fixes, the shortlog looked sane
and it merged and compiled cleanly.

Please note two other commits are still pending in drm-misc-fixes:

commit 745fd50f3b044db6a3922e1718306555613164b0
Author: Daniel Vetter 
Date:   Wed Jan 31 12:04:50 2018 +0100

drm/cirrus: Load lut in crtc_commit

Gustavo sent a pull request for this one on Jan 31 but erroneously
based it on the wrong commit and it ended up not being pulled by Dave.

commit 54f809cfbd6b4a43959039f5d33596ed3297ce16
Author: Leo (Sunpeng) Li 
Date:   Wed Jan 17 12:51:08 2018 +0100

drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits

This one has already been pulled by Dave into drm-next for 4.17
as commit 1c6c6ebb but Maarten subsequently cherry-picked
it onto drm-misc-fixes.

Let me know if I've made any mistakes.

Thanks,

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


[Intel-gfx] [PATCH igt 2/2] tests/gem_ctx_param: Update invalid param

2018-02-17 Thread Chris Wilson
From: Antonio Argenziano 

Since commit: drm/i915/scheduler: Support user-defined priorities, the
driver support an extra context param to set context's priority. Add
tests for that interface and update invalid tests.

v2:
- Add arg size validation test. (Chris)
- Add arg value overflow test. (Chris)
- Add test for unsupported platforms. (Chris)
- Feed interface with all priority values and in random order. (Chris)

v3:
- Parametrize tests. (Chris)

v4:
- Code-style refactoring. (Chris)

Signed-off-by: Antonio Argenziano 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Cc: Tvrtko Ursulin 
Reviewed-by: Chris Wilson 
---
 tests/gem_ctx_param.c | 152 +-
 1 file changed, 151 insertions(+), 1 deletion(-)

diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index db68f57b..6745b0f1 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -24,10 +24,119 @@
  *Daniel Vetter 
  */
 
+#include 
+#include 
+
 #include "igt.h"
 
 IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 
+#define BIT(x) (1ul << (x))
+
+#define NEW_CTXBIT(0)
+#define USER BIT(1)
+
+static int reopen_driver(int fd)
+{
+   char path[256];
+
+   snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+   fd = open(path, O_RDWR);
+   igt_assert_lte(0, fd);
+
+   return fd;
+}
+
+static void set_priority(int i915)
+{
+   static const int64_t test_values[] = {
+   /* Test space too big, pick significant values */
+   INT_MIN,
+
+   I915_CONTEXT_MIN_USER_PRIORITY - 1,
+   I915_CONTEXT_MIN_USER_PRIORITY,
+   I915_CONTEXT_MIN_USER_PRIORITY + 1,
+
+   I915_CONTEXT_DEFAULT_PRIORITY - 1,
+   I915_CONTEXT_DEFAULT_PRIORITY,
+   I915_CONTEXT_DEFAULT_PRIORITY + 1,
+
+   I915_CONTEXT_MAX_USER_PRIORITY - 1,
+   I915_CONTEXT_MAX_USER_PRIORITY,
+   I915_CONTEXT_MAX_USER_PRIORITY + 1,
+
+   INT_MAX
+   };
+   unsigned int size;
+   int64_t *values;
+
+   igt_require(getuid() == 0);
+
+   size = ARRAY_SIZE(test_values);
+   values = malloc(sizeof(test_values) * 2);
+   igt_assert(values);
+
+   for (unsigned i = 0; i < size; i++) {
+   values[i] = test_values[i];
+   values[i + size] = test_values[i] | (uint64_t)rand() << 32;
+   }
+
+   igt_permute_array(values, size, igt_exchange_int64);
+
+   igt_fork(flags, NEW_CTX | USER) {
+   int fd = reopen_driver(i915);
+   struct drm_i915_gem_context_param arg = {
+   .param = I915_CONTEXT_PARAM_PRIORITY,
+   .ctx_id = flags & NEW_CTX ? gem_context_create(fd) : 0,
+   };
+   int64_t old_prio;
+
+
+   if (flags & USER) {
+   igt_debug("Dropping root privilege\n");
+   igt_drop_root();
+   }
+
+   gem_context_get_param(fd, );
+   old_prio = arg.value;
+
+   for (unsigned i = 0; i < size; i++) {
+   int64_t prio = values[i];
+   int expected = 0;
+   int err;
+
+   arg.value = prio;
+
+   if (flags & USER &&
+   prio > I915_CONTEXT_DEFAULT_PRIORITY)
+   expected = -EPERM;
+
+   if (prio < I915_CONTEXT_MIN_USER_PRIORITY ||
+   prio > I915_CONTEXT_MAX_USER_PRIORITY)
+   expected = -EINVAL;
+
+   err =__gem_context_set_param(fd, );
+   igt_assert_f(err == expected,
+"Priority requested %" PRId64 ", expected 
result %d, returned %d\n",
+prio, expected, err);
+
+   gem_context_get_param(fd, );
+   if (!err)
+   old_prio = prio;
+   igt_assert_eq(arg.value, old_prio);
+   }
+
+   arg.value = 0;
+   gem_context_set_param(fd, );
+
+   if (flags & NEW_CTX)
+   gem_context_destroy(fd, arg.ctx_id);
+   }
+
+   igt_waitchildren();
+   free(values);
+}
+
 igt_main
 {
struct drm_i915_gem_context_param arg;
@@ -138,11 +247,52 @@ igt_main
gem_context_set_param(fd, );
}
 
+   arg.param = I915_CONTEXT_PARAM_PRIORITY;
+
+   

[Intel-gfx] [PATCH igt 1/2] lib/igt_aux: Add function to swap int64 in array

2018-02-17 Thread Chris Wilson
From: Antonio Argenziano 

v2: Use igt_swap()

Signed-off-by: Antonio Argenziano 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Reviewed-by: Chris Wilson 
---
 lib/igt_aux.c | 16 
 lib/igt_aux.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 8ca0b60d..a23375a4 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -577,6 +577,22 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
int_arr[j] = tmp;
 }
 
+/**
+ * igt_exchange_int64:
+ * @array: pointer to the array of int64_t
+ * @i: first position
+ * @j: second position
+ *
+ * Exchanges the two values at array indices @i and @j. Useful as an exchange
+ * function for igt_permute_array().
+ */
+void igt_exchange_int64(void *array, unsigned i, unsigned j)
+{
+   int64_t *a = array;
+
+   igt_swap(a[i], a[j]);
+}
+
 /**
  * igt_permute_array:
  * @array: pointer to array
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index f9c75992..43dd15fe 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -118,6 +118,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, 
bool interrupt);
for (struct timespec t__={}; igt_nsec_elapsed(__)>>20 < (t); )
 
 void igt_exchange_int(void *array, unsigned i, unsigned j);
+void igt_exchange_int64(void *array, unsigned i, unsigned j);
 void igt_permute_array(void *array, unsigned size,
   void (*exchange_func)(void *array,
 unsigned i,
-- 
2.16.1

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


[Intel-gfx] [PATCH igt] tests/gem_ctx_param: Update invalid param

2018-02-17 Thread Chris Wilson
From: Antonio Argenziano 

Since commit: drm/i915/scheduler: Support user-defined priorities, the
driver support an extra context param to set context's priority. Add
tests for that interface and update invalid tests.

v2:
- Add arg size validation test. (Chris)
- Add arg value overflow test. (Chris)
- Add test for unsupported platforms. (Chris)
- Feed interface with all priority values and in random order. (Chris)

v3:
- Parametrize tests. (Chris)

v4:
- Code-style refactoring. (Chris)

Signed-off-by: Antonio Argenziano 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Cc: Tvrtko Ursulin 
Reviewed-by: Chris Wilson 
---
 tests/gem_ctx_param.c | 152 +-
 1 file changed, 151 insertions(+), 1 deletion(-)

diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index db68f57b..6745b0f1 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -24,10 +24,119 @@
  *Daniel Vetter 
  */
 
+#include 
+#include 
+
 #include "igt.h"
 
 IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 
+#define BIT(x) (1ul << (x))
+
+#define NEW_CTXBIT(0)
+#define USER BIT(1)
+
+static int reopen_driver(int fd)
+{
+   char path[256];
+
+   snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+   fd = open(path, O_RDWR);
+   igt_assert_lte(0, fd);
+
+   return fd;
+}
+
+static void set_priority(int i915)
+{
+   static const int64_t test_values[] = {
+   /* Test space too big, pick significant values */
+   INT_MIN,
+
+   I915_CONTEXT_MIN_USER_PRIORITY - 1,
+   I915_CONTEXT_MIN_USER_PRIORITY,
+   I915_CONTEXT_MIN_USER_PRIORITY + 1,
+
+   I915_CONTEXT_DEFAULT_PRIORITY - 1,
+   I915_CONTEXT_DEFAULT_PRIORITY,
+   I915_CONTEXT_DEFAULT_PRIORITY + 1,
+
+   I915_CONTEXT_MAX_USER_PRIORITY - 1,
+   I915_CONTEXT_MAX_USER_PRIORITY,
+   I915_CONTEXT_MAX_USER_PRIORITY + 1,
+
+   INT_MAX
+   };
+   unsigned int size;
+   int64_t *values;
+
+   igt_require(getuid() == 0);
+
+   size = ARRAY_SIZE(test_values);
+   values = malloc(sizeof(test_values) * 2);
+   igt_assert(values);
+
+   for (unsigned i = 0; i < size; i++) {
+   values[i] = test_values[i];
+   values[i + size] = test_values[i] | (uint64_t)rand() << 32;
+   }
+
+   igt_permute_array(values, size, igt_exchange_int64);
+
+   igt_fork(flags, NEW_CTX | USER) {
+   int fd = reopen_driver(i915);
+   struct drm_i915_gem_context_param arg = {
+   .param = I915_CONTEXT_PARAM_PRIORITY,
+   .ctx_id = flags & NEW_CTX ? gem_context_create(fd) : 0,
+   };
+   int64_t old_prio;
+
+
+   if (flags & USER) {
+   igt_debug("Dropping root privilege\n");
+   igt_drop_root();
+   }
+
+   gem_context_get_param(fd, );
+   old_prio = arg.value;
+
+   for (unsigned i = 0; i < size; i++) {
+   int64_t prio = values[i];
+   int expected = 0;
+   int err;
+
+   arg.value = prio;
+
+   if (flags & USER &&
+   prio > I915_CONTEXT_DEFAULT_PRIORITY)
+   expected = -EPERM;
+
+   if (prio < I915_CONTEXT_MIN_USER_PRIORITY ||
+   prio > I915_CONTEXT_MAX_USER_PRIORITY)
+   expected = -EINVAL;
+
+   err =__gem_context_set_param(fd, );
+   igt_assert_f(err == expected,
+"Priority requested %" PRId64 ", expected 
result %d, returned %d\n",
+prio, expected, err);
+
+   gem_context_get_param(fd, );
+   if (!err)
+   old_prio = prio;
+   igt_assert_eq(arg.value, old_prio);
+   }
+
+   arg.value = 0;
+   gem_context_set_param(fd, );
+
+   if (flags & NEW_CTX)
+   gem_context_destroy(fd, arg.ctx_id);
+   }
+
+   igt_waitchildren();
+   free(values);
+}
+
 igt_main
 {
struct drm_i915_gem_context_param arg;
@@ -138,11 +247,52 @@ igt_main
gem_context_set_param(fd, );
}
 
+   arg.param = I915_CONTEXT_PARAM_PRIORITY;
+
+   

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] lib/igt_aux: Add function to swap int64 in array (rev2)

2018-02-17 Thread Patchwork
== Series Details ==

Series: series starting with [1/2] lib/igt_aux: Add function to swap int64 in 
array (rev2)
URL   : https://patchwork.freedesktop.org/series/38466/
State : failure

== Summary ==

IGT patchset tested on top of latest successful build
1aec09098d3c3729ed152aee9ee4e6e656fa7a3f igt/kms_frontbuffer_tracking: Disable 
FBC testing for -ENODEV

with latest DRM-Tip kernel build CI_DRM_3788
76724e6ea332 drm-tip: 2018y-02m-16d-21h-43m-36s UTC integration manifest

Testlist changes:
+igt@gem_ctx_param@get-priority-new-ctx
+igt@gem_ctx_param@set-priority-invalid-size
+igt@gem_ctx_param@set-priority-not-supported
+igt@gem_ctx_param@set-priority-range

Test gem_ctx_switch:
Subgroup basic-default-heavy:
incomplete -> PASS   (fi-cnl-y3)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass   -> INCOMPLETE (fi-snb-2520m) fdo#103713
pass   -> INCOMPLETE (fi-glk-1)

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-gvtdvmtotal:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  
time:426s
fi-blb-e6850 total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  
time:378s
fi-bsw-n3050 total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  
time:500s
fi-bwr-2160  total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 
time:291s
fi-bxt-dsi   total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  
time:485s
fi-bxt-j4205 total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  
time:485s
fi-byt-j1900 total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  
time:474s
fi-byt-n2820 total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  
time:460s
fi-cfl-s2total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  
time:566s
fi-cnl-y3total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  
time:583s
fi-elk-e7500 total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  
time:418s
fi-gdg-551   total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 
time:286s
fi-glk-1 total:245  pass:218  dwarn:0   dfail:0   fail:0   skip:26 
fi-hsw-4770  total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  
time:392s
fi-ilk-650   total:288  pass:227  dwarn:0   dfail:0   fail:1   skip:60  
time:414s
fi-ivb-3520m total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  
time:464s
fi-kbl-7500u total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  
time:469s
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:456s
fi-kbl-r total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  
time:503s
fi-pnv-d510  total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  
time:587s
fi-skl-6260u total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  
time:429s
fi-skl-6600u total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  
time:509s
fi-skl-6700hqtotal:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  
time:530s
fi-skl-6700k2total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  
time:498s
fi-skl-6770hqtotal:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  
time:473s
fi-skl-guc   total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  
time:416s
fi-skl-gvtdvmtotal:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  
time:434s
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:399s

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_942/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH igt] tests/gem_ctx_param: Update invalid param

2018-02-17 Thread Chris Wilson
From: Antonio Argenziano 

Since commit: drm/i915/scheduler: Support user-defined priorities, the
driver support an extra context param to set context's priority. Add
tests for that interface and update invalid tests.

v2:
- Add arg size validation test. (Chris)
- Add arg value overflow test. (Chris)
- Add test for unsupported platforms. (Chris)
- Feed interface with all priority values and in random order. (Chris)

v3:
- Parametrize tests. (Chris)

v4:
- Code-style refactoring. (Chris)

Signed-off-by: Antonio Argenziano 
Cc: Chris Wilson 
Cc: Michal Winiarski 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Cc: Tvrtko Ursulin 
---
 tests/gem_ctx_param.c | 152 +-
 1 file changed, 151 insertions(+), 1 deletion(-)

diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index db68f57b..6745b0f1 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -24,10 +24,119 @@
  *Daniel Vetter 
  */
 
+#include 
+#include 
+
 #include "igt.h"
 
 IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
 
+#define BIT(x) (1ul << (x))
+
+#define NEW_CTXBIT(0)
+#define USER BIT(1)
+
+static int reopen_driver(int fd)
+{
+   char path[256];
+
+   snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+   fd = open(path, O_RDWR);
+   igt_assert_lte(0, fd);
+
+   return fd;
+}
+
+static void set_priority(int i915)
+{
+   static const int64_t test_values[] = {
+   /* Test space too big, pick significant values */
+   INT_MIN,
+
+   I915_CONTEXT_MIN_USER_PRIORITY - 1,
+   I915_CONTEXT_MIN_USER_PRIORITY,
+   I915_CONTEXT_MIN_USER_PRIORITY + 1,
+
+   I915_CONTEXT_DEFAULT_PRIORITY - 1,
+   I915_CONTEXT_DEFAULT_PRIORITY,
+   I915_CONTEXT_DEFAULT_PRIORITY + 1,
+
+   I915_CONTEXT_MAX_USER_PRIORITY - 1,
+   I915_CONTEXT_MAX_USER_PRIORITY,
+   I915_CONTEXT_MAX_USER_PRIORITY + 1,
+
+   INT_MAX
+   };
+   unsigned int size;
+   int64_t *values;
+
+   igt_require(getuid() == 0);
+
+   size = ARRAY_SIZE(test_values);
+   values = malloc(sizeof(test_values) * 2);
+   igt_assert(values);
+
+   for (unsigned i = 0; i < size; i++) {
+   values[i] = test_values[i];
+   values[i + size] = test_values[i] | (uint64_t)rand() << 32;
+   }
+
+   igt_permute_array(values, size, igt_exchange_int64);
+
+   igt_fork(flags, NEW_CTX | USER) {
+   int fd = reopen_driver(i915);
+   struct drm_i915_gem_context_param arg = {
+   .param = I915_CONTEXT_PARAM_PRIORITY,
+   .ctx_id = flags & NEW_CTX ? gem_context_create(fd) : 0,
+   };
+   int64_t old_prio;
+
+
+   if (flags & USER) {
+   igt_debug("Dropping root privilege\n");
+   igt_drop_root();
+   }
+
+   gem_context_get_param(fd, );
+   old_prio = arg.value;
+
+   for (unsigned i = 0; i < size; i++) {
+   int64_t prio = values[i];
+   int expected = 0;
+   int err;
+
+   arg.value = prio;
+
+   if (flags & USER &&
+   prio > I915_CONTEXT_DEFAULT_PRIORITY)
+   expected = -EPERM;
+
+   if (prio < I915_CONTEXT_MIN_USER_PRIORITY ||
+   prio > I915_CONTEXT_MAX_USER_PRIORITY)
+   expected = -EINVAL;
+
+   err =__gem_context_set_param(fd, );
+   igt_assert_f(err == expected,
+"Priority requested %" PRId64 ", expected 
result %d, returned %d\n",
+prio, expected, err);
+
+   gem_context_get_param(fd, );
+   if (!err)
+   old_prio = prio;
+   igt_assert_eq(arg.value, old_prio);
+   }
+
+   arg.value = 0;
+   gem_context_set_param(fd, );
+
+   if (flags & NEW_CTX)
+   gem_context_destroy(fd, arg.ctx_id);
+   }
+
+   igt_waitchildren();
+   free(values);
+}
+
 igt_main
 {
struct drm_i915_gem_context_param arg;
@@ -138,11 +247,52 @@ igt_main
gem_context_set_param(fd, );
}
 
+   arg.param = I915_CONTEXT_PARAM_PRIORITY;
+
+   igt_subtest("set-priority-not-supported") {
+   

Re: [Intel-gfx] [PATCH 12/20] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-17 Thread Chris Wilson
Quoting Sagar Arun Kamble (2018-02-17 08:51:44)
> Earlier I had thought of calling ASSIGN_FW_DOMAINS_TABLE, 
> ASSIGN_*_MMIO_VFUNCS before intel_uncore_fw_domains_init
> and use I915_READ here for reading the fuse. But that approach seems to 
> expose the vfuncs and forcewake table before fw domains
> are initialized. Although we can get to know invalid access to 
> read/write accessors before fw_domains get initialized, current ordering
> of fw_domains init followed by fw domain table/read/write vfuncs init 
> seems right. So using fw_domains_get/put as suggested above
> should be the way.
> 
> Chris, Tvrtko, Mika, do you agree?

What's the complication with the fw_domains? Do the additional powerwell
depend on the fused status of the extra engines? Does it matter if the
fw_domain are prepped if they are never used?

Imo, I would have placed the fused discovery in
intel_engines_init_mmio() (where we do the setup and can take forcewake).
Then adding something like intel_uncore_reinit_mmio() (which would just
prune the uncore->fw_domains) after checking fused status with commentary
doesn't seem that horrible.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 12/20] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

2018-02-17 Thread Sagar Arun Kamble



On 2/13/2018 10:07 PM, Mika Kuoppala wrote:

From: Oscar Mateo 

In Gen11, the Video Decode engines (aka VDBOX, aka VCS, aka BSD) and the
Video Enhancement engines (aka VEBOX, aka VECS) could be fused off. Also,
each VDBOX and VEBOX has its own power well, which only exist if the related
engine exists in the HW.

Unfortunately, we have a Catch-22 situation going on: we need to read an
MMIO register with the fuse info, but we cannot fully enable MMIO until
we read it (since we need the real engines to initialize the forcewake
domains).
We need to ensure BLITTER is initialized first and use low level 
functions fw_domains_get/put()

around raw read to know these engines status.

  We workaround this problem by reading the fuse after the MMIO
is partially ready, but before we initialize forcewake.

Bspec: 20680

v2: We were shifting incorrectly for vebox disable (Vinay)

v3: Assert mmio is ready and warn if we have attempted to initialize
 forcewake for fused-off engines (Paulo)

v4:
   - Use INTEL_GEN in new code (Tvrtko)
   - Shorter local variable (Tvrtko, Michal)
   - Keep "if (!...) continue" style (Tvrtko)
   - No unnecessary BUG_ON (Tvrtko)
   - WARN_ON and cleanup if wrong mask (Tvrtko, Michal)
   - Use I915_READ_FW (Michal)
   - Use I915_MAX_VCS/VECS macros (Michal)

v5: Rebased by Rodrigo fixing conflicts on top of:
 commit 33def1ff7b0 ("drm/i915: Simplify intel_engines_init")

v6: Fix v5. Remove info->num_rings. (by Oscar)

v7: Rebase (Rodrigo).

v8:
   - s/intel_device_info_fused_off_engines/intel_device_info_init_mmio (Chris)
   - Make vdbox_disable & vebox_disable local variables (Chris)

Cc: Paulo Zanoni 
Cc: Vinay Belgaumkar 
Cc: Tvrtko Ursulin 
Cc: Michal Wajdeczko 
Cc: Chris Wilson 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Oscar Mateo 
---
  drivers/gpu/drm/i915/i915_drv.c  |  2 ++
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/i915_reg.h  |  5 +++
  drivers/gpu/drm/i915/intel_device_info.c | 54 
  4 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9380c9f69b0f..43b2f620bca7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1033,6 +1033,8 @@ static int i915_driver_init_mmio(struct drm_i915_private 
*dev_priv)
if (ret < 0)
goto err_bridge;
  
+	intel_device_info_init_mmio(dev_priv);

This should be called during intel_uncore_fw_domains_init after
        fw_domain_init(.., FW_DOMAIN_ID_BLITTER, ..);

+
intel_uncore_init(dev_priv);
  
  	intel_uc_init_mmio(dev_priv);

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 65e674668b2e..ba16c2025364 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3438,6 +3438,7 @@ void i915_unreserve_fence(struct drm_i915_fence_reg 
*fence);
  void i915_gem_revoke_fences(struct drm_i915_private *dev_priv);
  void i915_gem_restore_fences(struct drm_i915_private *dev_priv);
  
+void intel_device_info_init_mmio(struct drm_i915_private *dev_priv);

  void i915_gem_detect_bit_6_swizzle(struct drm_i915_private *dev_priv);
  void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj,
   struct sg_table *pages);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b6cd725ff0b7..2b8d3a13dd27 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2860,6 +2860,11 @@ enum i915_power_well_id {
  #define GEN10_EU_DISABLE3 _MMIO(0x9140)
  #define   GEN10_EU_DIS_SS_MASK0xff
  
+#define GEN11_GT_VEBOX_VDBOX_DISABLE	_MMIO(0x9140)

+#define GEN11_GT_VDBOX_DISABLE_MASK0xff
+#define GEN11_GT_VEBOX_DISABLE_SHIFT   16
+#define GEN11_GT_VEBOX_DISABLE_MASK(0xff << GEN11_GT_VEBOX_DISABLE_SHIFT)
+
  #define GEN6_BSD_SLEEP_PSMI_CONTROL   _MMIO(0x12050)
  #define   GEN6_BSD_SLEEP_MSG_DISABLE  (1 << 0)
  #define   GEN6_BSD_SLEEP_FLUSH_DISABLE(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
b/drivers/gpu/drm/i915/intel_device_info.c
index 9352f34e75c4..7c8779faf162 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -595,3 +595,57 @@ void intel_driver_caps_print(const struct 
intel_driver_caps *caps,
  {
drm_printf(p, "scheduler: %x\n", caps->scheduler);
  }
+
+/*
+ * Determine which engines are fused off in our particular hardware.
+ *
+ * This function needs to be called after the MMIO has been setup (as we need
+ * to read registers) but before uncore init (because the powerwell for the
+ * fused off engines doesn't exist, so we cannot initialize forcewake for them)
+ */