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

[Zhiyi Zhang]

Signed-off-by: Zhiyi Zhang 
---
 dlls/vulkan-1/tests/vulkan.c |   2 +-
 dlls/winex11.drv/vulkan.c    |  11 +++
 dlls/winex11.drv/xrandr.c    | 156 +++++++++++++++++++++++++++++++++++
 3 files changed, 168 insertions(+), 1 deletion(-)

diff --git a/dlls/vulkan-1/tests/vulkan.c b/dlls/vulkan-1/tests/vulkan.c
index 4ebfe6815bf..4f80a36cd1b 100644
--- a/dlls/vulkan-1/tests/vulkan.c
+++ b/dlls/vulkan-1/tests/vulkan.c
@@ -212,7 +212,7 @@ static void test_enumerate_physical_device2(void)
             trace("Device '%s', device UUID: %s, driver UUID: %s, device LUID: %08x:%08x.\n",
                   properties2.properties.deviceName, wine_dbgstr_guid((const GUID *)id.deviceUUID),
                   wine_dbgstr_guid((const GUID *)id.driverUUID), luid->HighPart, luid->LowPart);
-            todo_wine ok(id.deviceLUIDValid == VK_TRUE, "Expected valid device LUID.\n");
+            ok(id.deviceLUIDValid == VK_TRUE, "Expected valid device LUID.\n");
         }
     }

diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c
index 9ee79220f76..28c53a98d14 100644
--- a/dlls/winex11.drv/vulkan.c
+++ b/dlls/winex11.drv/vulkan.c
@@ -504,12 +504,21 @@ static VkResult X11DRV_vkGetPhysicalDevicePresentRectanglesKHR(VkPhysicalDevice
     return pvkGetPhysicalDevicePresentRectanglesKHR(phys_dev, x11_surface->surface, count, rects);
 }

+#if defined(SONAME_LIBXRANDR) && defined(HAVE_XRRGETPROVIDERRESOURCES)
+extern void fill_vk_device_luid_property(VkPhysicalDevice physical_device,
+        VkPhysicalDeviceProperties2 *properties) DECLSPEC_HIDDEN;
+#else
+#define fill_vk_device_luid_property(a, b)
+#endif
+
 static void X11DRV_vkGetPhysicalDeviceProperties2(VkPhysicalDevice phys_dev,
         VkPhysicalDeviceProperties2 *properties)
 {
     TRACE("%p, %p\n", phys_dev, properties);

pvkGetPhysicalDeviceProperties2(phys_dev, properties);
+
+    fill_vk_device_luid_property(phys_dev, properties);
 }

static void X11DRV_vkGetPhysicalDeviceProperties2KHR(VkPhysicalDevice phys_dev,
@@ -521,6 +530,8 @@ static void X11DRV_vkGetPhysicalDeviceProperties2KHR(VkPhysicalDevice phys_dev,
         pvkGetPhysicalDeviceProperties2KHR(phys_dev, properties);
     else
         pvkGetPhysicalDeviceProperties2(phys_dev, properties);
+
+    fill_vk_device_luid_property(phys_dev, properties);
 }

static VkResult X11DRV_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice phys_dev,
diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c
index ce7efc9deb4..161c053e659 100644
--- a/dlls/winex11.drv/xrandr.c
+++ b/dlls/winex11.drv/xrandr.c
@@ -35,8 +35,16 @@ WINE_DECLARE_DEBUG_CHANNEL(winediag);
 #include <X11/extensions/Xrandr.h>
 #include "x11drv.h"

+#define VK_NO_PROTOTYPES
+#define WINE_VK_HOST
+
+#include "winreg.h"
+#include "devguid.h"
+#include "setupapi.h"
 #include "wine/heap.h"
 #include "wine/unicode.h"
+#include "wine/vulkan.h"
+#include "wine/vulkan_driver.h"

static void *xrandr_handle;

@@ -680,6 +688,154 @@ static BOOL is_crtc_primary( RECT primary, const XRRCrtcInfo *crtc )
            crtc->y + crtc->height == primary.bottom;
 }

+#ifdef SONAME_LIBVULKAN
+
+VK_DEFINE_NON_DISPATCHABLE_HANDLE(VkDisplayKHR)
+DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0xf1, 0x73, 0xab, 0xad, 0x3e, 0xc6, 2);
+DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_RANDR_PROVIDER_ID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
+
+static BOOL get_provider_luid( RRProvider provider_id, LUID *luid )
+{
+    static const WCHAR pci[] = {'P','C','I',0};
+    SP_DEVINFO_DATA device_data;
+    DWORD type, device_idx = 0;
+    BOOL ret = FALSE;
+    HDEVINFO devinfo;
+    HANDLE mutex;
+    UINT64 id;
+
+    mutex = get_display_device_init_mutex();
+    devinfo = SetupDiGetClassDevsW( &GUID_DEVCLASS_DISPLAY, pci, NULL, 0 );
+    device_data.cbSize = sizeof(device_data);
+    while (SetupDiEnumDeviceInfo( devinfo, device_idx++, &device_data) )
+    {
+        if (!SetupDiGetDevicePropertyW( devinfo, &device_data,
+                                        &WINE_DEVPROPKEY_GPU_RANDR_PROVIDER_ID, &type, (BYTE *)&id,
+                                        sizeof(id), NULL, 0) )
+            continue;
+
+        if (id != provider_id)
+            continue;
+
+        if (SetupDiGetDevicePropertyW( devinfo, &device_data, &DEVPROPKEY_GPU_LUID, &type,
+                                       (BYTE *)luid, sizeof(*luid), NULL, 0) )
+        {
+            ret = TRUE;
+            break;
+        }
+    }
+    SetupDiDestroyDeviceInfoList( devinfo );
+    release_display_device_init_mutex( mutex );
+
+    if (!ret)
+        WARN("Failed to get LUID for RandR provider %#lx.\n", provider_id);
+    return ret;
+}
+
+static void set_luid_property( VkPhysicalDeviceProperties2 *properties, const LUID *luid )
+{
+    VkPhysicalDeviceIDProperties *id;
+    VkBaseOutStructure *header;
+
+    for (header = (VkBaseOutStructure *)properties; header; header = header->pNext)
+    {
+        if (header->sType != VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES)
+            continue;
+
+        id = (VkPhysicalDeviceIDProperties *)header;
+        memcpy( &id->deviceLUID, luid, sizeof(*luid) );
+        id->deviceLUIDValid = VK_TRUE;
+        TRACE("Set LUID %08x:%08x for Vulkan physical device properties %p.\n", luid->HighPart,
+               luid->LowPart, properties);
+        return;
+    }
+}
+
+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;
+
+            if (get_provider_luid( provider_resources->providers[provider_idx], &luid ))
+            {
+                set_luid_property( properties, &luid );
+                goto done;
+            }
+        }
+
+        pXRRFreeProviderInfo( provider_info );
+        provider_info = NULL;
+    }
+
+done:
+    if (provider_info)
+        pXRRFreeProviderInfo( provider_info );
+    if (provider_resources)
+        pXRRFreeProviderResources( provider_resources );
+    if (screen_resources)
+        pXRRFreeScreenResources( screen_resources );
+    vulkan_funcs->p_vkDestroyInstance( vk_instance, NULL );
+}
+#endif /* SONAME_LIBVULKAN */
+
 static BOOL xrandr14_get_gpus( struct x11drv_gpu **new_gpus, int *count )
 {
     static const WCHAR wine_adapterW[] = {'W','i','n','e',' ','A','d','a','p','t','e','r',0};

[Brendan Shanks]

> On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang  wrote:
> Signed-off-by: Zhiyi Zhang
>
> dlls/vulkan-1/tests/vulkan.c |   2 +-
> dlls/winex11.drv/vulkan.c    |  11 +++
> dlls/winex11.drv/xrandr.c    | 156 +++++++++++++++++++++++++++++++++++
> 3 files changed, 168 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c
> index ce7efc9deb4..161c053e659 100644
> --- a/dlls/winex11.drv/xrandr.c
> +++ b/dlls/winex11.drv/xrandr.c
> @@ -35,8 +35,16 @@ WINE_DECLARE_DEBUG_CHANNEL(winediag);
> #include <X11/extensions/Xrandr.h>
> #include "x11drv.h"
>
> +#define VK_NO_PROTOTYPES
> +#define WINE_VK_HOST
> -
>
> +#include "winreg.h"
> +#include "devguid.h"
> +#include "setupapi.h"
> #include "wine/heap.h"
> #include "wine/unicode.h"
> +#include "wine/vulkan.h"
> +#include "wine/vulkan_driver.h"
>
> static void *xrandr_handle;
>
> @@ -680,6 +688,154 @@ static BOOL is_crtc_primary( RECT primary, const XRRCrtcInfo *crtc )
>            crtc->y + crtc->height == primary.bottom;
> }
>
> +#ifdef SONAME_LIBVULKAN
> -
>
> +VK_DEFINE_NON_DISPATCHABLE_HANDLE(VkDisplayKHR)
> +DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0xf1, 0x73, 0xab, 0xad, 0x3e, 0xc6, 2);
> +DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_RANDR_PROVIDER_ID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
> -
>
> +static BOOL get_provider_luid( RRProvider provider_id, LUID *luid )
> +{
> - static const WCHAR pci[] = {'P','C','I',0};
> - SP_DEVINFO_DATA device_data;
> - DWORD type, device_idx = 0;
> - BOOL ret = FALSE;
> - HDEVINFO devinfo;
> - HANDLE mutex;
> - UINT64 id;
> -
> - mutex = get_display_device_init_mutex();
> - devinfo = SetupDiGetClassDevsW( &GUID_DEVCLASS_DISPLAY, pci, NULL, 0 );
> - device_data.cbSize = sizeof(device_data);
> - while (SetupDiEnumDeviceInfo( devinfo, device_idx++, &device_data) )
> - {
> -
>    if (!SetupDiGetDevicePropertyW( devinfo, &device_data,
> -
>                                    &WINE_DEVPROPKEY_GPU_RANDR_PROVIDER_ID, &type, (BYTE *)&id,
> -
>                                    sizeof(id), NULL, 0) )
> -
>        continue;
> -
> -
>    if (id != provider_id)
> -
>        continue;
> -
> -
>    if (SetupDiGetDevicePropertyW( devinfo, &device_data, &DEVPROPKEY_GPU_LUID, &type,
> -
>                                   (BYTE *)luid, sizeof(*luid), NULL, 0) )
> -
>    {
> -
>        ret = TRUE;
> -
>        break;
> -
>    }
> - }
> - SetupDiDestroyDeviceInfoList( devinfo );
> - release_display_device_init_mutex( mutex );
> -
> - if (!ret)
> -
>    WARN("Failed to get LUID for RandR provider %#lx.\n", provider_id);
> - return ret;
>
> +}
> -
>
> +static void set_luid_property( VkPhysicalDeviceProperties2 *properties, const LUID *luid )
> +{
> - VkPhysicalDeviceIDProperties *id;
> - VkBaseOutStructure *header;
> -
> - for (header = (VkBaseOutStructure *)properties; header; header = header->pNext)
> - {
> -
>    if (header->sType != VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES)
> -
>        continue;
> -
> -
>    id = (VkPhysicalDeviceIDProperties *)header;
> -
>    memcpy( &id->deviceLUID, luid, sizeof(*luid) );
> -
>    id->deviceLUIDValid = VK_TRUE;
> -
>    TRACE("Set LUID %08x:%08x for Vulkan physical device properties %p.\n", luid->HighPart,
> -
>           luid->LowPart, properties);

id->deviceNodeMask also must be set, probably to 1.

“If deviceLUIDValid is VK_TRUE, deviceNodeMask must contain exactly one bit. If Vulkan is running on an operating system that supports the Direct3D 12 API and physicalDevice corresponds to an individual device in a linked device adapter, deviceNodeMask identifies the Direct3D 12 node corresponding to physicalDevice. Otherwise, deviceNodeMask must be 1."
> -
>    return;
> - }
>
> +}
> -
>
> +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.

[Brendan Shanks]

> On Jun 12, 2020, at 12:04 PM, Brendan Shanks  wrote:
> > On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang  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.

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).

Brendan

[Zhiyi Zhang]

On 6/13/20 5:53 AM, Brendan Shanks wrote:
> > On Jun 12, 2020, at 12:04 PM, Brendan Shanks  wrote:
> > > On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang  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

[Zhiyi Zhang]

On 6/13/20 3:04 AM, Brendan Shanks wrote:
> > On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang  wrote:
> > Signed-off-by: Zhiyi Zhang
> >
> > dlls/vulkan-1/tests/vulkan.c |   2 +-
> > dlls/winex11.drv/vulkan.c    |  11 +++
> > dlls/winex11.drv/xrandr.c    | 156 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 168 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c
> > index ce7efc9deb4..161c053e659 100644
> > --- a/dlls/winex11.drv/xrandr.c
> > +++ b/dlls/winex11.drv/xrandr.c
> > @@ -35,8 +35,16 @@ WINE_DECLARE_DEBUG_CHANNEL(winediag);
> > #include <X11/extensions/Xrandr.h>
> > #include "x11drv.h"
> >
> > +#define VK_NO_PROTOTYPES
> > +#define WINE_VK_HOST
> > -
> >
> > +#include "winreg.h"
> > +#include "devguid.h"
> > +#include "setupapi.h"
> > #include "wine/heap.h"
> > #include "wine/unicode.h"
> > +#include "wine/vulkan.h"
> > +#include "wine/vulkan_driver.h"
> >
> > static void *xrandr_handle;
> >
> > @@ -680,6 +688,154 @@ static BOOL is_crtc_primary( RECT primary, const XRRCrtcInfo *crtc )
> >            crtc->y + crtc->height == primary.bottom;
> > }
> >
> > +#ifdef SONAME_LIBVULKAN
> > -
> >
> > +VK_DEFINE_NON_DISPATCHABLE_HANDLE(VkDisplayKHR)
> > +DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0xf1, 0x73, 0xab, 0xad, 0x3e, 0xc6, 2);
> > +DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_RANDR_PROVIDER_ID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
> > -
> >
> > +static BOOL get_provider_luid( RRProvider provider_id, LUID *luid )
> > +{
> > - static const WCHAR pci[] = {'P','C','I',0};
> > - SP_DEVINFO_DATA device_data;
> > - DWORD type, device_idx = 0;
> > - BOOL ret = FALSE;
> > - HDEVINFO devinfo;
> > - HANDLE mutex;
> > - UINT64 id;
> > -
> > - mutex = get_display_device_init_mutex();
> > - devinfo = SetupDiGetClassDevsW( &GUID_DEVCLASS_DISPLAY, pci, NULL, 0 );
> > - device_data.cbSize = sizeof(device_data);
> > - while (SetupDiEnumDeviceInfo( devinfo, device_idx++, &device_data) )
> > - {
> > -
> >    if (!SetupDiGetDevicePropertyW( devinfo, &device_data,
> > -
> >                                    &WINE_DEVPROPKEY_GPU_RANDR_PROVIDER_ID, &type, (BYTE *)&id,
> > -
> >                                    sizeof(id), NULL, 0) )
> > -
> >        continue;
> > -
> > -
> >    if (id != provider_id)
> > -
> >        continue;
> > -
> > -
> >    if (SetupDiGetDevicePropertyW( devinfo, &device_data, &DEVPROPKEY_GPU_LUID, &type,
> > -
> >                                   (BYTE *)luid, sizeof(*luid), NULL, 0) )
> > -
> >    {
> > -
> >        ret = TRUE;
> > -
> >        break;
> > -
> >    }
> > - }
> > - SetupDiDestroyDeviceInfoList( devinfo );
> > - release_display_device_init_mutex( mutex );
> > -
> > - if (!ret)
> > -
> >    WARN("Failed to get LUID for RandR provider %#lx.\n", provider_id);
> > - return ret;
> >
> > +}
> > -
> >
> > +static void set_luid_property( VkPhysicalDeviceProperties2 *properties, const LUID *luid )
> > +{
> > - VkPhysicalDeviceIDProperties *id;
> > - VkBaseOutStructure *header;
> > -
> > - for (header = (VkBaseOutStructure *)properties; header; header = header->pNext)
> > - {
> > -
> >    if (header->sType != VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES)
> > -
> >        continue;
> > -
> > -
> >    id = (VkPhysicalDeviceIDProperties *)header;
> > -
> >    memcpy( &id->deviceLUID, luid, sizeof(*luid) );
> > -
> >    id->deviceLUIDValid = VK_TRUE;
> > -
> >    TRACE("Set LUID %08x:%08x for Vulkan physical device properties %p.\n", luid->HighPart,
> > -
> >           luid->LowPart, properties);
>
> id->deviceNodeMask also must be set, probably to 1.
>
> “If deviceLUIDValid is VK_TRUE, deviceNodeMask must contain exactly one bit. If Vulkan is running on an operating system that supports the Direct3D 12 API and physicalDevice corresponds to an individual device in a linked device adapter, deviceNodeMask identifies the Direct3D 12 node corresponding to physicalDevice. Otherwise, deviceNodeMask must be 1."

I think deviceNodeMask is already set.
> > -
> >    return;
> > - }
> >
> > +}
> > -
> >
> > +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.

[Brendan Shanks]

> On Jun 15, 2020, at 7:07 AM, Zhiyi Zhang  wrote:
>
> On 6/13/20 3:04 AM, Brendan Shanks wrote:
> > > On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang  wrote:
> > >
> > > +static void set_luid_property( VkPhysicalDeviceProperties2 *properties, const LUID *luid )
> > > +{
> > > - VkPhysicalDeviceIDProperties *id;
> > > - VkBaseOutStructure *header;
> > > -
> > > - for (header = (VkBaseOutStructure *)properties; header; header = header->pNext)
> > > - {
> > > -
> > >    if (header->sType != VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES)
> > > -
> > >        continue;
> > > -
> > > -
> > >    id = (VkPhysicalDeviceIDProperties *)header;
> > > -
> > >    memcpy( &id->deviceLUID, luid, sizeof(*luid) );
> > > -
> > >    id->deviceLUIDValid = VK_TRUE;
> > > -
> > >    TRACE("Set LUID %08x:%08x for Vulkan physical device properties %p.\n", luid->HighPart,
> > > -
> > >           luid->LowPart, properties);
> >
> > id->deviceNodeMask also must be set, probably to 1.
> >
> > “If deviceLUIDValid is VK_TRUE, deviceNodeMask must contain exactly one bit. If Vulkan is running on an operating system that supports the Direct3D 12 API and physicalDevice corresponds to an individual device in a linked device adapter, deviceNodeMask identifies the Direct3D 12 node corresponding to physicalDevice. Otherwise, deviceNodeMask must be 1."
>
> I think deviceNodeMask is already set.

At least the NVIDIA binary driver does seem to set it to 1, but the spec doesn’t guarantee that: "If deviceLUIDValid is VK_FALSE, the values of deviceLUID and deviceNodeMask are undefined.”

Brendan

[Zhiyi Zhang]

On 6/17/20 7:04 AM, Brendan Shanks wrote:
> > On Jun 15, 2020, at 7:07 AM, Zhiyi Zhang  wrote:
> >
> > On 6/13/20 3:04 AM, Brendan Shanks wrote:
> > > > On Jun 12, 2020, at 12:21 AM, Zhiyi Zhang  wrote:
> > > >
> > > > +static void set_luid_property( VkPhysicalDeviceProperties2 *properties, const LUID *luid )
> > > > +{
> > > > - VkPhysicalDeviceIDProperties *id;
> > > > - VkBaseOutStructure *header;
> > > > -
> > > > - for (header = (VkBaseOutStructure *)properties; header; header = header->pNext)
> > > > - {
> > > > -
> > > >    if (header->sType != VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES)
> > > > -
> > > >        continue;
> > > > -
> > > > -
> > > >    id = (VkPhysicalDeviceIDProperties *)header;
> > > > -
> > > >    memcpy( &id->deviceLUID, luid, sizeof(*luid) );
> > > > -
> > > >    id->deviceLUIDValid = VK_TRUE;
> > > > -
> > > >    TRACE("Set LUID %08x:%08x for Vulkan physical device properties %p.\n", luid->HighPart,
> > > > -
> > > >           luid->LowPart, properties);
> > >
> > > id->deviceNodeMask also must be set, probably to 1.
> > >
> > > “If deviceLUIDValid is VK_TRUE, deviceNodeMask must contain exactly one bit. If Vulkan is running on an operating system that supports the Direct3D 12 API and physicalDevice corresponds to an individual device in a linked device adapter, deviceNodeMask identifies the Direct3D 12 node corresponding to physicalDevice. Otherwise, deviceNodeMask must be 1."
> >
> > I think deviceNodeMask is already set.
>
> At least the NVIDIA binary driver does seem to set it to 1, but the spec doesn’t guarantee that: "If deviceLUIDValid is VK_FALSE, the values of deviceLUID and deviceNodeMask are undefined.”
>
> Brendan

Yeah, I set it to 1 in v2 just to be safe.

Thanks,
Zhiyi