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

[Iago Toral]

On Mon, 2018-03-12 at 07:37 -0700, Jason Ekstrand wrote:
> On Mon, Mar 12, 2018 at 1:40 AM, Iago Toral Quiroga <itoral at igalia.co
> m> 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.
> > 
> > 
> > 
> > Also see the public issue:
> > 
> > https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/is
> > sues/2323
> > 
> > 
> > 
> > v2:
> > 
> >   - Include reference to github issue (Emil)
> > 
> >   - Rebased on top of Vulkan 1.1 changes.
> > 
> > 
> > 
> > Reviewed-by: Emil Velikov <emil.velikov at collabora.com> (v1)
> > 
> > ---
> > 
> > 
> > 
> > Emil, I had to rebase the patch on top of Jason's 1.1 changes. He
> > had already
> > 
> > accounted for device dispatches in that work, so now I just build
> > on top of
> > 
> > that now. With that, I am not sure whether the comment you were
> > asking for makes
> > 
> > sense in this patch any more (I think it should have gone in
> > Jason's, when he
> > 
> > added is_device_entrypoint()). I you want a comment for that I can
> > send
> > 
> > another patch to include it, or maybe ammend the first patch in
> > this series to
> > 
> > include that. However, do notice that the comment you were
> > referring to has
> > 
> > been removed from the spec, since now it is clearly stated that
> > only
> > 
> > core device-level commands return non-NULL pointers, so I think my
> > preference
> > 
> > would be to not add ny comments.
> > 
> > 
> > 
> > Also, this version won't do for 18.0, but I guess we can still use
> > the
> > 
> > previous version for that if you want to put it in.
> > 
> > 
> > 
> >  src/intel/vulkan/anv_entrypoints_gen.py | 6 +++++-
> > 
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > 
> > 
> > diff --git a/src/intel/vulkan/anv_entrypoints_gen.py
> > b/src/intel/vulkan/anv_entrypoints_gen.py
> > 
> > index 2dddd7f1aa..8e0d036469 100644
> > 
> > --- a/src/intel/vulkan/anv_entrypoints_gen.py
> > 
> > +++ b/src/intel/vulkan/anv_entrypoints_gen.py
> > 
> > @@ -274,7 +274,11 @@ anv_entrypoint_is_enabled(int index, uint32_t
> > core_version,
> > 
> >     case ${e.num}:
> > 
> >        /* ${e.name} */
> > 
> >     % if e.core_version:
> > 
> > -      return ${e.core_version.c_vk_version()} <= core_version;
> > 
> > +      % if not e.is_device_entrypoint():
> > 
> > +         return !device && ${e.core_version.c_vk_version()} <=
> > core_version;
> > 
> > +      % else:
> > 
> > +         return ${e.core_version.c_vk_version()} <= core_version;
> > 
> > +      % endif
> 
> Any particular reason why you did "if not" instead of just flipping
> the then and else blocks?

Not really, I can change that. 
> >     % elif e.extensions:
> > 
> >       % for ext in e.extensions:
> > 
> >         % if ext.type == 'instance':
> > 
> > --
> > 
> > 2.14.1
> > 
> > 
> > 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180313/fb9641d7/attachment-0001.html>