[Mesa-stable] [Mesa-dev] [PATCH] anv/entrypoints: VkGetDeviceProcAddr returns NULL for core instance commands

[Emil Velikov]

Hi Iago,

Top level questions:

I think this and the original commit should go to stable right?
The dispatch in 17.3 is very different making, yet 18.0 should be
perfectly fine.

Mildly related - anv is missing a special case for following three
extensions. Should we port those over from radv?

vkCreateInstance vkEnumerateInstanceExtensionProperties
vkEnumerateInstanceLayerProperties

Bas, you might want to port these over?

On 5 March 2018 at 10:46, Iago Toral Quiroga <itoral at igalia.com> wrote:
> af5f2322d0c64 addressed this for extension commands, but the spec mandates
> this behavior also for core API commands. From the Vulkan spec,
> Table 2. vkGetDeviceProcAddr behavior:
>
> device     pname                            return
> ----------------------------------------------------------
> (..)
> device     core device-level command        fp
> (...)
>
> See that it specifically states "device-level".
>
> Since the vk.xml file doesn't state if core commands are instance or
> device level, we identify device level commands as the ones that take a
> VkDevice, VkQueue or VkCommandBuffer as their first parameter.
>
> Fixes test failures in new work-in-progress CTS tests.
AFAICT this and the original patch are in-light of the following
github issue, right?
If so, please add it to the commit message.

https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/issues/2323

> ---
>  src/intel/vulkan/anv_entrypoints_gen.py | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_entrypoints_gen.py b/src/intel/vulkan/anv_entrypoints_gen.py
> index 34ffedb116..f0bfc3b6f8 100644
> --- a/src/intel/vulkan/anv_entrypoints_gen.py
> +++ b/src/intel/vulkan/anv_entrypoints_gen.py
> @@ -222,7 +222,11 @@ anv_entrypoint_is_enabled(int index, uint32_t core_version,
>  % for e in entrypoints:
>     case ${e.num}:
>     % if e.core_version:
> -      return ${e.core_version.c_vk_version()} <= core_version;
> +      % if e.instance_dispatch:
> +         return !device && ${e.core_version.c_vk_version()} <= core_version;
> +      % else:
> +         return ${e.core_version.c_vk_version()} <= core_version;
> +      % endif
>     % elif e.extension:
>        % if e.extension.type == 'instance':
>        return !device && instance->${e.extension.name[3:]};
> @@ -350,6 +354,12 @@ def cal_hash(name):
>
>  EntrypointParam = namedtuple('EntrypointParam', 'type name decl')
>
> +DeviceChildrenList = ['VkDevice', 'VkQueue', 'VkCommandBuffer' ]
> +
> +def is_device_child(name):
> +   """Tell if the given type name is a VkDevice or one of its children."""
> +   return name in DeviceChildrenList
> +
>  class Entrypoint(object):
>      def __init__(self, name, return_type, params, guard = None):
>          self.name = name
> @@ -358,6 +368,7 @@ class Entrypoint(object):
>          self.guard = guard
>          self.enabled = False
>          self.num = None
> +        self.instance_dispatch = not is_device_child(params[0].type)
On a quick look this seems odd - one is interested in instance
dispatch, yet checking for device.
There are some subtleties about it in the above mentioned report.

Please add a small comment about those.

With the link + trivial comment the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil