Re: [PATCH 00/10] winex11: Add Vulkan ICD.
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The amount of code sharing between ICDs, I expect to be mostly in generated code and maybe a few helper functions. For now I think it is best to duplicate code and longer-term if a little bit of sharing makes sense put this in a staticly linked helper library. I want to avoid implementing our own 'winevulkan' glue ICD library in between loader and display driver as I fear this will cause many issues, because the object design didn't lend for this (assumes 1 ICD = 1 driver). Besides some applications load the ICD directly.
This first wave of patches gets some initial ICD infrastructure in place. Enough to create a minimal Vulkan Instance and get some function loading going. It will take another 6-7 patches or so before 'vulkaninfo' the Vulkan equivalent of glxinfo will work to some degree.
For now the easiest way to load the ICD is to add a registry key: [HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\Drivers\] "C:\Windows\System32\winex11.json"=dword:00000000
winex11.json: { "file_format_version": "1.0.0", "ICD": { "library_path": "c:\\windows\\system32\\winex11.drv", "api_version": "1.0.51" } }
How would this be able to support multiple drivers with the same install? It seems to me that an intermediate library that calls the currently active driver would be better. What sort of issues do you think it would cause? -- Alexandre Julliard julliard(a)winehq.org
On Wed, Nov 1, 2017 at 11:26 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The amount of code sharing between ICDs, I expect to be mostly in generated code and maybe a few helper functions. For now I think it is best to duplicate code and longer-term if a little bit of sharing makes sense put this in a staticly linked helper library. I want to avoid implementing our own 'winevulkan' glue ICD library in between loader and display driver as I fear this will cause many issues, because the object design didn't lend for this (assumes 1 ICD = 1 driver). Besides some applications load the ICD directly.
This first wave of patches gets some initial ICD infrastructure in place. Enough to create a minimal Vulkan Instance and get some function loading going. It will take another 6-7 patches or so before 'vulkaninfo' the Vulkan equivalent of glxinfo will work to some degree.
For now the easiest way to load the ICD is to add a registry key: [HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\Drivers\] "C:\Windows\System32\winex11.json"=dword:00000000
winex11.json: { "file_format_version": "1.0.0", "ICD": { "library_path": "c:\\windows\\system32\\winex11.drv", "api_version": "1.0.51" } }
How would this be able to support multiple drivers with the same install? It seems to me that an intermediate library that calls the currently active driver would be better. What sort of issues do you think it would cause?
In the current approach our Vulkan ICD uses the Linux Vulkan loader internally. The Linux Vulkan loader should discover all drivers. In other words, our Vulkan ICD may expose multiple Vulkan physical devices.
Józef Kucia <joseph.kucia(a)gmail.com> writes:
On Wed, Nov 1, 2017 at 11:26 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The amount of code sharing between ICDs, I expect to be mostly in generated code and maybe a few helper functions. For now I think it is best to duplicate code and longer-term if a little bit of sharing makes sense put this in a staticly linked helper library. I want to avoid implementing our own 'winevulkan' glue ICD library in between loader and display driver as I fear this will cause many issues, because the object design didn't lend for this (assumes 1 ICD = 1 driver). Besides some applications load the ICD directly.
This first wave of patches gets some initial ICD infrastructure in place. Enough to create a minimal Vulkan Instance and get some function loading going. It will take another 6-7 patches or so before 'vulkaninfo' the Vulkan equivalent of glxinfo will work to some degree.
For now the easiest way to load the ICD is to add a registry key: [HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\Drivers\] "C:\Windows\System32\winex11.json"=dword:00000000
winex11.json: { "file_format_version": "1.0.0", "ICD": { "library_path": "c:\\windows\\system32\\winex11.drv", "api_version": "1.0.51" } }
How would this be able to support multiple drivers with the same install? It seems to me that an intermediate library that calls the currently active driver would be better. What sort of issues do you think it would cause?
In the current approach our Vulkan ICD uses the Linux Vulkan loader internally. The Linux Vulkan loader should discover all drivers. In other words, our Vulkan ICD may expose multiple Vulkan physical devices.
I mean multiple Wine drivers. For instance, once we have a Wayland driver, a proper Wine package would include both Wayland and X11 drivers, and they would be selected at run-time based on the DISPLAY variable. So it seems to me the Vulkan ICD should retrieve the current driver from the HWND, similar to what opengl32.dll is doing. -- Alexandre Julliard julliard(a)winehq.org
On Wed, Nov 1, 2017 at 11:44 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Józef Kucia <joseph.kucia(a)gmail.com> writes:
On Wed, Nov 1, 2017 at 11:26 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The amount of code sharing between ICDs, I expect to be mostly in generated code and maybe a few helper functions. For now I think it is best to duplicate code and longer-term if a little bit of sharing makes sense put this in a staticly linked helper library. I want to avoid implementing our own 'winevulkan' glue ICD library in between loader and display driver as I fear this will cause many issues, because the object design didn't lend for this (assumes 1 ICD = 1 driver). Besides some applications load the ICD directly.
This first wave of patches gets some initial ICD infrastructure in place. Enough to create a minimal Vulkan Instance and get some function loading going. It will take another 6-7 patches or so before 'vulkaninfo' the Vulkan equivalent of glxinfo will work to some degree.
For now the easiest way to load the ICD is to add a registry key: [HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\Drivers\] "C:\Windows\System32\winex11.json"=dword:00000000
winex11.json: { "file_format_version": "1.0.0", "ICD": { "library_path": "c:\\windows\\system32\\winex11.drv", "api_version": "1.0.51" } }
How would this be able to support multiple drivers with the same install? It seems to me that an intermediate library that calls the currently active driver would be better. What sort of issues do you think it would cause?
In the current approach our Vulkan ICD uses the Linux Vulkan loader internally. The Linux Vulkan loader should discover all drivers. In other words, our Vulkan ICD may expose multiple Vulkan physical devices.
I mean multiple Wine drivers. For instance, once we have a Wayland driver, a proper Wine package would include both Wayland and X11 drivers, and they would be selected at run-time based on the DISPLAY variable. So it seems to me the Vulkan ICD should retrieve the current driver from the HWND, similar to what opengl32.dll is doing.
I misunderstood the question and followed with the hasty reply. The current approach would provide a multiple ICDs in this case, and the Vulkan loader would try to load all Wine display drivers which are registered as Vulkan ICDs. That's an issue definitely.
On Wed, Nov 1, 2017 at 3:44 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Józef Kucia <joseph.kucia(a)gmail.com> writes:
On Wed, Nov 1, 2017 at 11:26 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The amount of code sharing between ICDs, I expect to be mostly in generated code and maybe a few helper functions. For now I think it is best to duplicate code and longer-term if a little bit of sharing makes sense put this in a staticly linked helper library. I want to avoid implementing our own 'winevulkan' glue ICD library in between loader and display driver as I fear this will cause many issues, because the object design didn't lend for this (assumes 1 ICD = 1 driver). Besides some applications load the ICD directly.
This first wave of patches gets some initial ICD infrastructure in place. Enough to create a minimal Vulkan Instance and get some function loading going. It will take another 6-7 patches or so before 'vulkaninfo' the Vulkan equivalent of glxinfo will work to some degree.
For now the easiest way to load the ICD is to add a registry key: [HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\Drivers\] "C:\Windows\System32\winex11.json"=dword:00000000
winex11.json: { "file_format_version": "1.0.0", "ICD": { "library_path": "c:\\windows\\system32\\winex11.drv", "api_version": "1.0.51" } }
How would this be able to support multiple drivers with the same install? It seems to me that an intermediate library that calls the currently active driver would be better. What sort of issues do you think it would cause?
In the current approach our Vulkan ICD uses the Linux Vulkan loader internally. The Linux Vulkan loader should discover all drivers. In other words, our Vulkan ICD may expose multiple Vulkan physical devices.
I mean multiple Wine drivers. For instance, once we have a Wayland driver, a proper Wine package would include both Wayland and X11 drivers, and they would be selected at run-time based on the DISPLAY variable. So it seems to me the Vulkan ICD should retrieve the current driver from the HWND, similar to what opengl32.dll is doing.
There is not guaranteed to be an HWND, you can use Vulkan for offscreen rendering with no interaction with X11 / Win32 concepts. Any driver like enumeration is triggered through some of the 'global vulkan calls' e.g. vkEnumerateInstanceProperties / vkEnumeratePhysicalDevices. It is more about adapter enumeration across drivers. To make things more fun in some use cases you can use multiple GPUs across different drivers to work together, to some extend managed by the loader.
On Wed, Nov 1, 2017 at 3:26 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The amount of code sharing between ICDs, I expect to be mostly in generated code and maybe a few helper functions. For now I think it is best to duplicate code and longer-term if a little bit of sharing makes sense put this in a staticly linked helper library. I want to avoid implementing our own 'winevulkan' glue ICD library in between loader and display driver as I fear this will cause many issues, because the object design didn't lend for this (assumes 1 ICD = 1 driver). Besides some applications load the ICD directly.
This first wave of patches gets some initial ICD infrastructure in place. Enough to create a minimal Vulkan Instance and get some function loading going. It will take another 6-7 patches or so before 'vulkaninfo' the Vulkan equivalent of glxinfo will work to some degree.
For now the easiest way to load the ICD is to add a registry key: [HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\Drivers\] "C:\Windows\System32\winex11.json"=dword:00000000
winex11.json: { "file_format_version": "1.0.0", "ICD": { "library_path": "c:\\windows\\system32\\winex11.drv", "api_version": "1.0.51" } }
How would this be able to support multiple drivers with the same install? It seems to me that an intermediate library that calls the currently active driver would be better. What sort of issues do you think it would cause?
-- Alexandre Julliard julliard(a)winehq.org
Let me share a link on the overall design of Vulkan, which I think is helpful: https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/blob/master... Regarding ICD loading, there are multiple mechanisms. So far I used the 'simple' method of adding a static registry key, but the loader also supports a dynamic way see the section 'ICD Discovery on Windows'. We can probably make that method work as well and deal with winex11, wineandroid reporting the appropriate keys at runtime if they support this capability on a given system. Vulkan was designed outside of Win32 and doesn't use native types (e.g. HWND). Compared to OpenGL the API has no state. All the state is passed in through objects to the API calls, there is no 'active context' like in OpenGL through like a wglMakeCurrent. The first parameter (e.g. VkInstance / VkDevice) is a 'dispatchable object' which means it contains a jump table and the ICD performs some magic around that too. One other thing to realize is that Vulkan is next to an OpenGL replacement, kind of an OpenCL replacement too, you are not required to use windows and can do offscreen rendering. The role of the Vulkan loader is to manage all the different ICDs on the system and inject the layers at the correct points. My fear is that if we add our own 'winevulkan' library in between it will become another loader. For APIs such as vkEnumeratePhysicalDevices or vkEnumerateInstanceExtensionProperties we would need to aggregate the values across winex11, winewayland, etc, which is what the loader does for us. This is painful and we need to figure ways ourselves to load said driver. Another potential area of concern is function pointer loading, see the whole function loading chain and terminator concept in the doc. Basically there are a couple of loading calls (vkGetInstanceProcAddr, vkGetDeviceProcAddr and others). Each of these GetProcAddr calls you can hook into at a different layer in the stack (could be a 'layer' or at the bottom the ICD). If you load functions at a high layer, the functions can be more generic and dispatch to multiple ICDs if needed, but the lower you get the more device / driver specific a function call with the 'same name' becomes. I'm worried about assumptions by the loader causing confusion (function pointer compatiblity / object type compatibility) if we are hiding multiple ICDs behind what it thinks the ICD is. Overall the ICD loading issues can be solved. At some point we may want our own vulkan loader (vulkan-1.dll). This is somewhat tricky as the loader is doing a lot of work and we should not recreate it since it is opensource. The only problem is the license of the loader. Until recently it was BSD licensed, so we could have pulled it in. Right now it is Apache license. The range of options include: 1) Fork BSD licensed loader and keep it up to date. 2) Supply loader through a winegecko / mono kind of way 3) Add Apache licensed vulkan-1 loader to our codebase and keep that directory Apache licensed.
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The role of the Vulkan loader is to manage all the different ICDs on the system and inject the layers at the correct points. My fear is that if we add our own 'winevulkan' library in between it will become another loader. For APIs such as vkEnumeratePhysicalDevices or vkEnumerateInstanceExtensionProperties we would need to aggregate the values across winex11, winewayland, etc, which is what the loader does for us. This is painful and we need to figure ways ourselves to load said driver.
On the contrary, you don't want to aggregate the values. It wouldn't make sense to load both winex11 and winewayland, you can only use the one that corresponds to what user32 is currently using for that process. So a separate winevulkan would simply query the driver corresponding to the desktop window, and rely on user32 to load it. And for the offscreen rendering case, it wouldn't even have to go through the graphics driver at all. Your approach would require all the Wine graphics drivers to support being loaded behind user32's back, and refuse to work if they aren't the active driver for the current desktop. That's a lot of complication, for no benefit that I can see. Trying to force a 1:1 mapping between the Vulkan ICD loader and the user32 graphics drivers would be a mistake IMO. -- Alexandre Julliard julliard(a)winehq.org
On Wed, Nov 1, 2017 at 3:14 PM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Roderick Colenbrander <thunderbird2k(a)gmail.com> writes:
The role of the Vulkan loader is to manage all the different ICDs on the system and inject the layers at the correct points. My fear is that if we add our own 'winevulkan' library in between it will become another loader. For APIs such as vkEnumeratePhysicalDevices or vkEnumerateInstanceExtensionProperties we would need to aggregate the values across winex11, winewayland, etc, which is what the loader does for us. This is painful and we need to figure ways ourselves to load said driver.
On the contrary, you don't want to aggregate the values. It wouldn't make sense to load both winex11 and winewayland, you can only use the one that corresponds to what user32 is currently using for that process. So a separate winevulkan would simply query the driver corresponding to the desktop window, and rely on user32 to load it. And for the offscreen rendering case, it wouldn't even have to go through the graphics driver at all.
I need to think a bit on what could be done in such an approach. In OpenGL terms there is not a clear separation between the GL calls and the WGL aspects, they are intertwined. Building a winevulkan requires untwinining these aspects, I'm not sure how cleanly that can be done as the APIs weren't designed for it. Which is why I hoped to go the Vulkan PNP registry way (like vulkan windows operates). One example of the intertwining. One of the 'contexts' created in Vulkan is a VkInstance through vkCreateInstance call. Vulkan loves passing massive data structures through. If the application requests the surface extension, it wants graphics rendering else it may just want to do compute. To solve this situation, I would need to parse the extension field and pass it to winex11 and let it do a native vkCreateInstance call and do its X11 magic OR or if no graphics just finish completion of the call in winevulkan and have it create the native instance by calling vulkan itself. There are likely other calls, which which need to passed through to winex11 or not based on parameter parsing. Function call loading is another thing. Both winevulkan and winex11 would need to make Vulkan calls. Either both libraries load them or one passes a function table (with potentially wayland/x11/..) specific calls back. The higher and lower vulkan layers will be fairly intimate. There are subfields in Vulkan objects (VkInstance, VkDevice) which a winex11 may fill or if no graphics just winevulkan. Generic Vulkan calls like vkEnumeratePhysicalDevices or other calls need to operate on them.
Your approach would require all the Wine graphics drivers to support being loaded behind user32's back, and refuse to work if they aren't the active driver for the current desktop. That's a lot of complication, for no benefit that I can see. Trying to force a 1:1 mapping between the Vulkan ICD loader and the user32 graphics drivers would be a mistake IMO.
I see your concerns. We could prevent loading of a graphics driver if it wasn't picked. Luckily there are only 2 entry points to ICDs. Another approach as I said is to use the PNP related registry keys, which only loaded display drivers can set. Overall my main concern is that we need to figure out how to bend Vulkan into being like a GL and WGL, while the APIs weren't designed for it. There will be a large amount of intimacy between the layers just because of shared data structures different APIs need to act.
participants (3)
-
Alexandre Julliard -
Józef Kucia -
Roderick Colenbrander