On 01/22/2018 09:01 PM, Jason Ekstrand wrote:
On Mon, Jan 22, 2018 at 4:29 AM, Tapani Pälli <tapani.pa...@intel.com
<mailto:tapani.pa...@intel.com>> wrote:
v2: add ANV_CONTEXT_REALTIME_PRIORITY (Chris)
use unreachable with unknown priority (Samuel)
v3: add stubs in gem_stubs.c (Emil)
use priority defines from gen_defines.h
Signed-off-by: Tapani Pälli <tapani.pa...@intel.com
<mailto:tapani.pa...@intel.com>>
Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com
<mailto:sigles...@igalia.com>> (v2)
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk
<mailto:ch...@chris-wilson.co.uk>> (v2)
---
src/intel/vulkan/anv_device.c | 25 +++++++++++++++++++
src/intel/vulkan/anv_extensions.py | 2 ++
src/intel/vulkan/anv_gem.c | 51
++++++++++++++++++++++++++++++++++++++
src/intel/vulkan/anv_gem_stubs.c | 10 ++++++++
src/intel/vulkan/anv_private.h | 3 +++
5 files changed, 91 insertions(+)
diff --git a/src/intel/vulkan/anv_device.c
b/src/intel/vulkan/anv_device.c
index 777abd8757..42ebc19f2b 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -369,6 +369,9 @@ anv_physical_device_init(struct
anv_physical_device *device,
device->has_syncobj_wait = device->has_syncobj &&
anv_gem_supports_syncobj_wait(fd);
+ if (anv_gem_has_context_priority(fd))
+ device->has_context_priority = true;
+
bool swizzled = anv_gem_get_bit6_swizzle(fd, I915_TILING_X);
/* Starting with Gen10, the timestamp frequency of the command
streamer may
@@ -1205,6 +1208,15 @@ VkResult anv_CreateDevice(
}
}
+ /* Check if client specified queue priority. */
+ const VkDeviceQueueGlobalPriorityCreateInfoEXT *queue_priority =
+ vk_find_struct_const(pCreateInfo->pQueueCreateInfos[0].pNext,
+
DEVICE_QUEUE_GLOBAL_PRIORITY_CREATE_INFO_EXT);
+
+ VkQueueGlobalPriorityEXT priority =
+ queue_priority ? queue_priority->globalPriority :
+ VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT;
+
device = vk_alloc2(&physical_device->instance->alloc, pAllocator,
sizeof(*device), 8,
VK_SYSTEM_ALLOCATION_SCOPE_DEVICE);
@@ -1234,6 +1246,19 @@ VkResult anv_CreateDevice(
goto fail_fd;
}
+ /* As per spec, the driver implementation may deny requests to
acquire
+ * a priority above the default priority (MEDIUM) if the caller
does not
+ * have sufficient privileges. In this scenario
VK_ERROR_NOT_PERMITTED_EXT
+ * is returned.
+ */
+ if (physical_device->has_context_priority) {
+ int err = anv_gem_set_context_priority(device, priority);
+ if (err != 0 && priority > VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT) {
+ result = vk_error(VK_ERROR_NOT_PERMITTED_EXT);
+ goto fail_fd;
+ }
+ }
+
device->info = physical_device->info;
device->isl_dev = physical_device->isl_dev;
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index adfebca985..aacf39248f 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -86,6 +86,8 @@ EXTENSIONS = [
Extension('VK_KHX_multiview', 1, True),
Extension('VK_EXT_debug_report', 8, True),
Extension('VK_EXT_external_memory_dma_buf', 1, True),
+ Extension('VK_EXT_global_priority', 1,
+ 'device->has_context_priority'),
]
class VkVersion:
diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c
index 34c0989108..7f83820429 100644
--- a/src/intel/vulkan/anv_gem.c
+++ b/src/intel/vulkan/anv_gem.c
@@ -30,6 +30,7 @@
#include <fcntl.h>
#include "anv_private.h"
+#include "common/gen_defines.h"
static int
anv_ioctl(int fd, unsigned long request, void *arg)
@@ -302,6 +303,56 @@ close_and_return:
return swizzled;
}
+static int
+vk_priority_to_anv(int priority)
+{
+ switch (priority) {
+ case VK_QUEUE_GLOBAL_PRIORITY_LOW_EXT:
+ return GEN_CONTEXT_LOW_PRIORITY;
+ case VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT:
+ return GEN_CONTEXT_MEDIUM_PRIORITY;
+ case VK_QUEUE_GLOBAL_PRIORITY_HIGH_EXT:
+ return GEN_CONTEXT_HIGH_PRIORITY;
+ case VK_QUEUE_GLOBAL_PRIORITY_REALTIME_EXT:
+ return GEN_CONTEXT_REALTIME_PRIORITY;
+ default:
+ unreachable("Invalid priority");
+ }
+}
I think I'd rather have the conversion in anv_device.c and just make the
anv_gem functions take an i915 priority.
ok will change that
Other than that, and a couple other nits, this looks good to me.
One other question, do we have tests? I quickly searched the piglit
list and didn't see any. Writing a crucible test shouldn't be that
I wrote a small API test here:
https://cgit.freedesktop.org/~tpalli/crucible/commit/?h=VK_EXT_global_priority
hard. You just have to submit a bunch of command buffers and show that
they get re-ordered to favor the higher-priority context. You could do
that with a bunch of compute shader invocations that "take a number"
from a shared atomic or something like that.
I think overall problem with complex run time test is that spec is
written like 'will attempt' and 'may retain' which means driver does not
promise to do anything. So basically we can test it, but test must pass
whatever happens.
Having said that I've had plans to finish my EGL test for same
functionality. Plan there was to queue some complex work for 2 different
priority contexts and then query time used to complete work
(TIME_ELAPSED_EXT) and hopefully this would tell if higher priority
wins. Do you think similar test would work with Vulkan (or at all)?
The current func.sync.semaphore-fd tests should also probably be
modified to use it. They currently use queue priorities in an attempt
to make things to out-of-sync and, when I was testing semaphores, I had
that hooked up to the context priority. Now that there's a proper
extension for this, we should use that.
ok will take a look at this one.
+
+static int
+_anv_gem_set_context_priority(int fd,
+ int context_id,
+ int priority)
+{
+ struct drm_i915_gem_context_param p = {
+ .ctx_id = context_id,
+ .param = I915_CONTEXT_PARAM_PRIORITY,
+ .value = vk_priority_to_anv(priority),
+ };
+ int err = 0;
+
+ if (anv_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, &p))
+ err = -errno;
+
+ return err;
+}
We may as well make this anv_gem_set_context_param, similar to
anv_gem_get_context_param. That way we can re-use it easier in the future.
+
+int
+anv_gem_set_context_priority(struct anv_device *device,
+ int priority)
+{
+ return _anv_gem_set_context_priority(device->fd, device->context_id,
+ priority);
+}
+
+bool
+anv_gem_has_context_priority(int fd)
+{
+ return !_anv_gem_set_context_priority(fd, 0,
+
VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT);
+}
+
int
anv_gem_create_context(struct anv_device *device)
{
diff --git a/src/intel/vulkan/anv_gem_stubs.c
b/src/intel/vulkan/anv_gem_stubs.c
index 26eb5c8a61..ad27e877a2 100644
--- a/src/intel/vulkan/anv_gem_stubs.c
+++ b/src/intel/vulkan/anv_gem_stubs.c
@@ -152,6 +152,16 @@ anv_gem_get_context_param(int fd, int context,
uint32_t param, uint64_t *value)
unreachable("Unused");
}
+bool anv_gem_has_context_priority(int fd)
+{
+ unreachable("Unused");
+}
+
+int anv_gem_set_context_priority(struct anv_device *device, int
priority)
+{
+ unreachable("Unused");
+}
+
int
anv_gem_get_aperture(int fd, uint64_t *size)
{
diff --git a/src/intel/vulkan/anv_private.h
b/src/intel/vulkan/anv_private.h
index ed711e9434..d3f74eb084 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -768,6 +768,7 @@ struct anv_physical_device {
bool has_exec_fence;
bool has_syncobj;
bool has_syncobj_wait;
+ bool has_context_priority;
uint32_t eu_total;
uint32_t subslice_total;
@@ -914,6 +915,8 @@ int anv_gem_execbuffer(struct anv_device *device,
int anv_gem_set_tiling(struct anv_device *device, uint32_t gem_handle,
uint32_t stride, uint32_t tiling);
int anv_gem_create_context(struct anv_device *device);
+bool anv_gem_has_context_priority(int fd);
+int anv_gem_set_context_priority(struct anv_device *device, int
priority);
int anv_gem_destroy_context(struct anv_device *device, int context);
int anv_gem_get_context_param(int fd, int context, uint32_t param,
uint64_t *value);
--
2.14.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
