[PATCH 4/4] winex11.drv: Fill Vulkan device LUID property.

[Zhiyi Zhang]

On 6/13/20 5:53 AM, Brendan Shanks wrote:
>> On Jun 12, 2020, at 12:04 PM, Brendan Shanks <bshanks at codeweavers.com> wrote:
>>
>>> On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang <zzhang at codeweavers.com> wrote:
>>> +
>>> +void fill_vk_device_luid_property( VkPhysicalDevice physical_device,
>>> +                                   VkPhysicalDeviceProperties2 *properties )
>>> +{
>>> +    VkResult (*p_vkGetRandROutputDisplayEXT)( VkPhysicalDevice, Display *, RROutput, VkDisplayKHR * );
>>> +    const struct vulkan_funcs *vulkan_funcs = get_vulkan_driver( WINE_VULKAN_DRIVER_VERSION );
>>> +    static const char *extensions[] = { "VK_EXT_acquire_xlib_display" };
>>> +    XRRProviderResources *provider_resources = NULL;
>>> +    XRRScreenResources *screen_resources = NULL;
>>> +    XRRProviderInfo *provider_info = NULL;
>>> +    unsigned int provider_idx, output_idx;
>>> +    VkInstanceCreateInfo create_info;
>>> +    VkDisplayKHR vk_display;
>>> +    VkInstance vk_instance;
>>> +    VkResult vr;
>>> +    LUID luid;
>>> +
>>> +    memset( &create_info, 0, sizeof(create_info) );
>>> +    create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO;
>>> +    create_info.enabledExtensionCount = ARRAY_SIZE(extensions);
>>> +    create_info.ppEnabledExtensionNames = extensions;
>>> +
>>> +    vr = vulkan_funcs->p_vkCreateInstance( &create_info, NULL, &vk_instance );
>>> +    if (vr != VK_SUCCESS)
>>> +    {
>>> +        ERR("Failed to create a Vulkan instance, vr %d.\n", vr);
>>> +        return;
>>> +    }
>>> +
>>> +    p_vkGetRandROutputDisplayEXT = (void *)vulkan_funcs->p_vkGetInstanceProcAddr( vk_instance, "vkGetRandROutputDisplayEXT" );
>>> +    if (!p_vkGetRandROutputDisplayEXT)
>>> +    {
>>> +        ERR("Failed to load vkGetRandROutputDisplayEXT.\n");
>>> +        goto done;
>>> +    }
>>> +
>>> +    screen_resources = xrandr_get_screen_resources();
>>> +    if (!screen_resources)
>>> +        goto done;
>>> +
>>> +    provider_resources = pXRRGetProviderResources( gdi_display, root_window );
>>> +    if (!provider_resources)
>>> +        goto done;
>>> +
>>> +    if (!provider_resources->nproviders && get_provider_luid( 1, &luid ))
>>> +    {
>>> +        set_luid_property( properties, &luid );
>>> +        goto done;
>>> +    }
>>> +
>>> +    for (provider_idx = 0; provider_idx < provider_resources->nproviders; ++provider_idx)
>>> +    {
>>> +        provider_info = pXRRGetProviderInfo( gdi_display, screen_resources,
>>> +                                             provider_resources->providers[provider_idx] );
>>> +        if (!provider_info)
>>> +            continue;
>>> +
>>> +        for (output_idx = 0; output_idx < provider_info->noutputs; ++output_idx)
>>> +        {
>>> +            vr = p_vkGetRandROutputDisplayEXT( physical_device, gdi_display,
>>> +                                               provider_info->outputs[output_idx], &vk_display);
>>> +            if (vr != VK_SUCCESS || vk_display == VK_NULL_HANDLE)
>>> +                continue;
>> I tried these patches out but they aren’t working on my system. vkGetRandROutputDisplayEXT() is called for each output, rrOutput is unique and seems valid, but the returned vk_display is VK_NULL_HANDLE. This is on NVIDIA with driver 440.82.
>>
>> I also tried calling vkGetPhysicalDeviceDisplayPropertiesKHR() here, it returns success but never fills in pPropertyCount or any valid pProperties. Adding VK_KHR_display as an extension also doesn’t help.
>> Something's going wrong here but I don’t know what it is, I’ll test these APIs out from a regular Linux binary.
> OK, I think I figured it out.
>
> Running my Win32 test app with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation complained that VK_EXT_direct_mode_display and VK_EXT_acquire_xlib_display were needed for vkGetRandROutputDisplayEXT(), and neither was being enabled.
VK_EXT_direct_mode_display is indeed needed. Thanks.

>
> My test app doesn’t enable VK_KHR_get_physical_device_properties2, so VK_EXT_acquire_xlib_display wasn’t being added.
> I think VK_KHR_get_physical_device_properties2 was promoted to core Vulkan 1.1, so the extension isn’t needed anymore. At least, KHRONOS_validation doesn’t complain about it, and vkGetPhysicalDeviceProperties2() works correctly.
> Any Vulkan experts know for sure?
>
> After forcing VK_EXT_acquire_xlib_display to be added to extensions, vkGetRandROutputDisplayEXT() works correctly and the LUID gets added. 
>
> This would be an advantage of my deviceUUID idea, since all the vkGetRandROutputDisplayEXT() calls are done early, extensions don’t need to be added to the app’s instance.
>
> Also, KHRONOS_validation reports that the VkDisplayKHR needs to be destroyed before the instance can be destroyed (with vkReleaseDisplayEXT() I believe).
This one seems to be a false positive in VK_LAYER_KHRONOS_validation. See https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/1660.
I tried export VK_INSTANCE_LAYERS=VK_LAYER_LUNARG_standard_validation:VK_LAYER_GOOGLE_threading:VK_LAYER_GOOGLE_unique_objects:VK_LAYER_LUNARG_core_validation:VK_LAYER_LUNARG_object_tracker:VK_LAYER_LUNARG_parameter_validation
with Vulkan SDK 1.1.101 and it doesn't complain about vkReleaseDisplayEXT not destroyed. Nor vkGetRandROutputDisplayEXT say anything about destroying the object.  vkReleaseDisplayEXT is used to release the display from acquired state, not the object itself.
Anyway, I also gave vkReleaseDisplayEXT a try and the errors still exist.

Thanks,
Zhiyi
 
>
> Brendan