<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 22, 2018 at 4:29 AM, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">v2: add ANV_CONTEXT_REALTIME_PRIORITY (Chris)<br>
use unreachable with unknown priority (Samuel)<br>
<br>
v3: add stubs in gem_stubs.c (Emil)<br>
use priority defines from gen_defines.h<br>
<br>
Signed-off-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>><br>
Reviewed-by: Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> (v2)<br>
Reviewed-by: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> (v2)<br>
---<br>
src/intel/vulkan/anv_device.c | 25 +++++++++++++++++++<br>
src/intel/vulkan/anv_<wbr>extensions.py | 2 ++<br>
src/intel/vulkan/anv_gem.c | 51 ++++++++++++++++++++++++++++++<wbr>++++++++<br>
src/intel/vulkan/anv_gem_<wbr>stubs.c | 10 ++++++++<br>
src/intel/vulkan/anv_private.h | 3 +++<br>
5 files changed, 91 insertions(+)<br>
<br>
diff --git a/src/intel/vulkan/anv_device.<wbr>c b/src/intel/vulkan/anv_device.<wbr>c<br>
index 777abd8757..42ebc19f2b 100644<br>
--- a/src/intel/vulkan/anv_device.<wbr>c<br>
+++ b/src/intel/vulkan/anv_device.<wbr>c<br>
@@ -369,6 +369,9 @@ anv_physical_device_init(<wbr>struct anv_physical_device *device,<br>
device->has_syncobj_wait = device->has_syncobj &&<br>
anv_gem_supports_syncobj_wait(<wbr>fd);<br>
<br>
+ if (anv_gem_has_context_priority(<wbr>fd))<br>
+ device->has_context_priority = true;<br>
+<br>
bool swizzled = anv_gem_get_bit6_swizzle(fd, I915_TILING_X);<br>
<br>
/* Starting with Gen10, the timestamp frequency of the command streamer may<br>
@@ -1205,6 +1208,15 @@ VkResult anv_CreateDevice(<br>
}<br>
}<br>
<br>
+ /* Check if client specified queue priority. */<br>
+ const VkDeviceQueueGlobalPriorityCre<wbr>ateInfoEXT *queue_priority =<br>
+ vk_find_struct_const(<wbr>pCreateInfo-><wbr>pQueueCreateInfos[0].pNext,<br>
+ DEVICE_QUEUE_GLOBAL_PRIORITY_<wbr>CREATE_INFO_EXT);<br>
+<br>
+ VkQueueGlobalPriorityEXT priority =<br>
+ queue_priority ? queue_priority->globalPriority :<br>
+ VK_QUEUE_GLOBAL_PRIORITY_<wbr>MEDIUM_EXT;<br>
+<br>
device = vk_alloc2(&physical_device-><wbr>instance->alloc, pAllocator,<br>
sizeof(*device), 8,<br>
VK_SYSTEM_ALLOCATION_SCOPE_<wbr>DEVICE);<br>
@@ -1234,6 +1246,19 @@ VkResult anv_CreateDevice(<br>
goto fail_fd;<br>
}<br>
<br>
+ /* As per spec, the driver implementation may deny requests to acquire<br>
+ * a priority above the default priority (MEDIUM) if the caller does not<br>
+ * have sufficient privileges. In this scenario VK_ERROR_NOT_PERMITTED_EXT<br>
+ * is returned.<br>
+ */<br>
+ if (physical_device->has_context_<wbr>priority) {<br>
+ int err = anv_gem_set_context_priority(<wbr>device, priority);<br>
+ if (err != 0 && priority > VK_QUEUE_GLOBAL_PRIORITY_<wbr>MEDIUM_EXT) {<br>
+ result = vk_error(VK_ERROR_NOT_<wbr>PERMITTED_EXT);<br>
+ goto fail_fd;<br>
+ }<br>
+ }<br>
+<br>
device->info = physical_device->info;<br>
device->isl_dev = physical_device->isl_dev;<br>
<br>
diff --git a/src/intel/vulkan/anv_<wbr>extensions.py b/src/intel/vulkan/anv_<wbr>extensions.py<br>
index adfebca985..aacf39248f 100644<br>
--- a/src/intel/vulkan/anv_<wbr>extensions.py<br>
+++ b/src/intel/vulkan/anv_<wbr>extensions.py<br>
@@ -86,6 +86,8 @@ EXTENSIONS = [<br>
Extension('VK_KHX_multiview', 1, True),<br>
Extension('VK_EXT_debug_<wbr>report', 8, True),<br>
Extension('VK_EXT_external_<wbr>memory_dma_buf', 1, True),<br>
+ Extension('VK_EXT_global_<wbr>priority', 1,<br>
+ 'device->has_context_priority'<wbr>),<br>
]<br>
<br>
class VkVersion:<br>
diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c<br>
index 34c0989108..7f83820429 100644<br>
--- a/src/intel/vulkan/anv_gem.c<br>
+++ b/src/intel/vulkan/anv_gem.c<br>
@@ -30,6 +30,7 @@<br>
#include <fcntl.h><br>
<br>
#include "anv_private.h"<br>
+#include "common/gen_defines.h"<br>
<br>
static int<br>
anv_ioctl(int fd, unsigned long request, void *arg)<br>
@@ -302,6 +303,56 @@ close_and_return:<br>
return swizzled;<br>
}<br>
<br>
+static int<br>
+vk_priority_to_anv(int priority)<br>
+{<br>
+ switch (priority) {<br>
+ case VK_QUEUE_GLOBAL_PRIORITY_LOW_<wbr>EXT:<br>
+ return GEN_CONTEXT_LOW_PRIORITY;<br>
+ case VK_QUEUE_GLOBAL_PRIORITY_<wbr>MEDIUM_EXT:<br>
+ return GEN_CONTEXT_MEDIUM_PRIORITY;<br>
+ case VK_QUEUE_GLOBAL_PRIORITY_HIGH_<wbr>EXT:<br>
+ return GEN_CONTEXT_HIGH_PRIORITY;<br>
+ case VK_QUEUE_GLOBAL_PRIORITY_<wbr>REALTIME_EXT:<br>
+ return GEN_CONTEXT_REALTIME_PRIORITY;<br>
+ default:<br>
+ unreachable("Invalid priority");<br>
+ }<br>
+}<br></blockquote><div><br></div><div>I think I'd rather have the conversion in anv_device.c and just make the anv_gem functions take an i915 priority.</div><div><br></div><div>Other than that, and a couple other nits, this looks good to me.</div><div><br></div><div>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 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.</div><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+static int<br>
+_anv_gem_set_context_<wbr>priority(int fd,<br>
+ int context_id,<br>
+ int priority)<br>
+{<br>
+ struct drm_i915_gem_context_param p = {<br>
+ .ctx_id = context_id,<br>
+ .param = I915_CONTEXT_PARAM_PRIORITY,<br>
+ .value = vk_priority_to_anv(priority),<br>
+ };<br>
+ int err = 0;<br>
+<br>
+ if (anv_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_<wbr>SETPARAM, &p))<br>
+ err = -errno;<br>
+<br>
+ return err;<br>
+}<br></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+int<br>
+anv_gem_set_context_priority(<wbr>struct anv_device *device,<br>
+ int priority)<br>
+{<br>
+ return _anv_gem_set_context_priority(<wbr>device->fd, device->context_id,<br>
+ priority);<br>
+}<br>
+<br>
+bool<br>
+anv_gem_has_context_priority(<wbr>int fd)<br>
+{<br>
+ return !_anv_gem_set_context_<wbr>priority(fd, 0,<br>
+ VK_QUEUE_GLOBAL_PRIORITY_<wbr>MEDIUM_EXT);<br>
+}<br>
+<br>
int<br>
anv_gem_create_context(struct anv_device *device)<br>
{<br>
diff --git a/src/intel/vulkan/anv_gem_<wbr>stubs.c b/src/intel/vulkan/anv_gem_<wbr>stubs.c<br>
index 26eb5c8a61..ad27e877a2 100644<br>
--- a/src/intel/vulkan/anv_gem_<wbr>stubs.c<br>
+++ b/src/intel/vulkan/anv_gem_<wbr>stubs.c<br>
@@ -152,6 +152,16 @@ anv_gem_get_context_param(int fd, int context, uint32_t param, uint64_t *value)<br>
unreachable("Unused");<br>
}<br>
<br>
+bool anv_gem_has_context_priority(<wbr>int fd)<br>
+{<br>
+ unreachable("Unused");<br>
+}<br>
+<br>
+int anv_gem_set_context_priority(<wbr>struct anv_device *device, int priority)<br>
+{<br>
+ unreachable("Unused");<br>
+}<br>
+<br>
int<br>
anv_gem_get_aperture(int fd, uint64_t *size)<br>
{<br>
diff --git a/src/intel/vulkan/anv_<wbr>private.h b/src/intel/vulkan/anv_<wbr>private.h<br>
index ed711e9434..d3f74eb084 100644<br>
--- a/src/intel/vulkan/anv_<wbr>private.h<br>
+++ b/src/intel/vulkan/anv_<wbr>private.h<br>
@@ -768,6 +768,7 @@ struct anv_physical_device {<br>
bool has_exec_fence;<br>
bool has_syncobj;<br>
bool has_syncobj_wait;<br>
+ bool has_context_priority;<br>
<br>
uint32_t eu_total;<br>
uint32_t subslice_total;<br>
@@ -914,6 +915,8 @@ int anv_gem_execbuffer(struct anv_device *device,<br>
int anv_gem_set_tiling(struct anv_device *device, uint32_t gem_handle,<br>
uint32_t stride, uint32_t tiling);<br>
int anv_gem_create_context(struct anv_device *device);<br>
+bool anv_gem_has_context_priority(<wbr>int fd);<br>
+int anv_gem_set_context_priority(<wbr>struct anv_device *device, int priority);<br>
int anv_gem_destroy_context(struct anv_device *device, int context);<br>
int anv_gem_get_context_param(int fd, int context, uint32_t param,<br>
uint64_t *value);<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.14.3<br>
<br>
</font></span></blockquote></div><br></div></div>