[Mesa-dev] [PATCH] anv: implement VK_EXT_global_priority extension
Tapani Pälli
tapani.palli at intel.com
Fri Jan 19 11:34:16 UTC 2018
On 01/19/2018 01:20 PM, Samuel Iglesias Gonsálvez wrote:
>
>
> On 19/01/18 12:11, Tapani Pälli wrote:
>>
>>
>> On 01/19/2018 01:08 PM, Samuel Iglesias Gonsálvez wrote:
>>>
>>>
>>> On 19/01/18 11:44, Tapani Pälli wrote:
>>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>>> ---
>>>>
>>>> Small crucible test available here:
>>>> https://cgit.freedesktop.org/~tpalli/crucible/commit/?h=VK_EXT_global_priority
>>>>
>>>>
>>>> src/intel/vulkan/anv_device.c | 25 +++++++++++++++++++
>>>> src/intel/vulkan/anv_extensions.py | 2 ++
>>>> src/intel/vulkan/anv_gem.c | 49
>>>> ++++++++++++++++++++++++++++++++++++++
>>>> src/intel/vulkan/anv_private.h | 7 ++++++
>>>> 4 files changed, 83 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..46069dcdc7 100644
>>>> --- a/src/intel/vulkan/anv_gem.c
>>>> +++ b/src/intel/vulkan/anv_gem.c
>>>> @@ -302,6 +302,55 @@ close_and_return:
>>>> return swizzled;
>>>> }
>>>> +static int
>>>> +vk_priority_to_anv(int priority)
>>>> +{
>>>> + switch (priority) {
>>>> + case VK_QUEUE_GLOBAL_PRIORITY_LOW_EXT:
>>>> + return ANV_CONTEXT_LOW_PRIORITY;
>>>> + case VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT:
>>>> + return ANV_CONTEXT_MEDIUM_PRIORITY;
>>>> + case VK_QUEUE_GLOBAL_PRIORITY_HIGH_EXT:
>>>> + return ANV_CONTEXT_HIGH_PRIORITY;
>>>> + case VK_QUEUE_GLOBAL_PRIORITY_REALTIME_EXT:
>>>> + default:
>>>> + return -1;
>>>
>>> Is this right? We are returning -1 on
>>> VK_QUEUE_GLOBAL_PRIORITY_REALTIME_EX, which should be the highest
>>> priority, according to the spec.
>>>
>>> Checking Intel kernel driver's sources, it checks the given priority in
>>> struct drm_i915_gem_context_param is within the range of
>>> I915_CONTEXT_MAX_USER_PRIORITY and I915_CONTEXT_MIN_USER_PRIORITY, whose
>>> values are 1023 and -1023, respectively. So at the end, you are setting
>>> the priority to be one unit lower than I915_CONTEXT_DEFAULT_PRIORITY.
>>>
>>> What do you want to do here? Give more priority to the realtime case or
>>> indicate that it is not supported?
>>
>> Argh, yep intention was to say it is not supported. I've not changed
>> it for ANV_CONTEXT_REALTIME_PRIORITY but I guess default case should
>> be something else, maybe just set medium?
>>
>>
> For the realtime case: you can do Chris solution or set it to a wrong
> value (x > MAX for example) to indicate it is not supported.
>
> For the default case: no other values are supported in the enum now, so
> you can either add an unreachable() or just return medium. Probably
> unreachable is better because there could be new priorities and we are
> sure we detect them to fix it here.
Thanks, I'll send v2 with these changes.
> Sam
>
>>> Sam
>>>> + }
>>>> +}
>>>> +
>>>> +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;
>>>> +}
>>>> +
>>>> +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_private.h
>>>> b/src/intel/vulkan/anv_private.h
>>>> index ed711e9434..caea3d96b4 100644
>>>> --- a/src/intel/vulkan/anv_private.h
>>>> +++ b/src/intel/vulkan/anv_private.h
>>>> @@ -106,6 +106,10 @@ struct gen_l3_config;
>>>> #define ANV_SVGS_VB_INDEX MAX_VBS
>>>> #define ANV_DRAWID_VB_INDEX (MAX_VBS + 1)
>>>> +#define ANV_CONTEXT_LOW_PRIORITY
>>>> ((I915_CONTEXT_MIN_USER_PRIORITY-1)/2)
>>>> +#define ANV_CONTEXT_MEDIUM_PRIORITY (I915_CONTEXT_DEFAULT_PRIORITY)
>>>> +#define ANV_CONTEXT_HIGH_PRIORITY
>>>> ((I915_CONTEXT_MAX_USER_PRIORITY+1)/2)
>>>> +
>>>> #define anv_printflike(a, b) __attribute__((__format__(__printf__,
>>>> a, b)))
>>>> static inline uint32_t
>>>> @@ -768,6 +772,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 +919,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);
>>>
>>>
>>
>
More information about the mesa-dev
mailing list