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

[Emil Velikov]

On 13 March 2018 at 15:02, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Tue, Mar 13, 2018 at 7:56 AM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
>>
>> On 12 March 2018 at 08:40, 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.
>> >
>> > Also see the public issue:
>> >
>> > https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/issues/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.
>> >
>> The suggestion was aimed as a reference, for anyone who missed the
>> specific hunk in the spec update.
>> There should be none, so yeah - don't bother ;-)
>>
>> > 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.
>> >
>> I would love to apply that one, if you don't mind.
>
>
> I would rather we not back-port this to 18.0 at least not without
> significant testing.  From what I've heard from the WG, it looks lie the two
> id games will be broken with it and the loader is not yet patched with a
> work-around.
>
Guess I have misunderstood Lenny's reply [1] - it indicated that
Wolfenstein/Doom are fine with said change.
Dully noted, I _might_ bring this up _only_ after checking that the
games continue to work correctly.

Thanks!
Emil

[1] https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/issues/2323#issuecomment-367810941