On Mon, Jan 22, 2018 at 4:29 AM, Tapani Pälli <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> > Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com> (v2) > Reviewed-by: Chris Wilson <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. 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 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. 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. > + > +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