Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added OpenCL Universal Driver Support for Win10 RS3 #21

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

ofirc
Copy link
Contributor

@ofirc ofirc commented Jul 21, 2017

Starting from Windows 10 RS3+ Graphics Drivers are no longer allowed to
write HKLM entries, and hence the current registry key:
HKLM\SOFTWARE\Khronos\OpenCL\Vendors

Is no longer applicable to graphics drivers for registering their ICD.
Instead, graphics drivers can write to either of the 2 registry keys
below:

  1. Display Adapter HKR
  2. Software Components HKR

Graphics Drivers (starting from RS3) have dedicated component INF files and are
referenced via the CopyINF directive and/or (optional) "INF AddComponent directive".

Every such component, e.g. OpenCL instance (ICD), is assigned a unique device
instance ID (e.g. 0000, 0001, etc) under the "Software Component" GUID and use it to
store the path to its ICD.

Therefore we enumerate ICDs in the following places (and also in the following order):

  1. Display Adapters HKRs (using the display adapter GUID)
  2. Software Component HKRs (using the CM_* APIs to traverse the device tree)
  3. The usual "Vendors" registry key (see above)

@ofirc
Copy link
Contributor Author

ofirc commented Jul 21, 2017

@bashbaug @kepatil
This is just a preliminary PR to provide some time ahead for reviewing the code.
The code above was approved by Microsoft for enumerating the ICD entries on Win10 RS3.

There are some opens until we can call this complete:

  1. Currently the ICD Loader doesn't link against OneCore.lib and hence is not fully Win10 compliant
  2. Needs to be massively tested and see that we did not cause any regressions with the new code
    (only minimal testing has been performed so far)

I will update further soon.

Thanks,
Ofir

@ofirc
Copy link
Contributor Author

ofirc commented Jul 21, 2017

@ofirc ofirc changed the title Added OpenCL Universal Driver Support for Win10 RS3 [WIP] Added OpenCL Universal Driver Support for Win10 RS3 Jul 21, 2017
Starting from Windows 10 RS3+ Graphics Drivers are no longer allowed to
write HKLM entries, and hence the current registry key:
HKLM\SOFTWARE\Khronos\OpenCL\Vendors

Is no longer applicable to graphics drivers for registering their ICD.
Instead, graphics drivers can write to either of the 2 registry keys
below:
1) Display Adapter HKR
2) Software Components HKR

Graphics Drivers (starting from RS3) have dedicated component INF files and are
referenced via the CopyINF directive and/or (optional) "INF AddComponent directive".

Every such component, e.g. OpenCL instance (ICD), is assigned a unique device
instance ID (e.g. 0000, 0001, etc) under the "Software Component" GUID and use it to
store the path to its ICD.

Therefore we enumerate ICDs in the following places (and also in the following order):
1) Display Adapters HKRs (using the display adapter GUID)
2) Software Component HKRs (using the CM_* APIs to traverse the device tree)
3) The usual "Vendors" registry key (see above)
@ofirc ofirc changed the title [WIP] Added OpenCL Universal Driver Support for Win10 RS3 Added OpenCL Universal Driver Support for Win10 RS3 Aug 3, 2017
@ofirc ofirc changed the title Added OpenCL Universal Driver Support for Win10 RS3 [WIP] Added OpenCL Universal Driver Support for Win10 RS3 Aug 3, 2017
@ofirc ofirc changed the title [WIP] Added OpenCL Universal Driver Support for Win10 RS3 Added OpenCL Universal Driver Support for Win10 RS3 Aug 3, 2017
@ofirc
Copy link
Contributor Author

ofirc commented Aug 3, 2017

@bashbaug @kepatil
Hi Guys,
Important sidenote:
Microsoft suggestion for RS3 is to make "Component INFs" for Vulkan and OpenCL.
OpenGL and Vulkan will stay in "Base INF", but migrate keys to HKR (instead of HKLM), similar to the requirement from OpenCL.

Now that it has been tested more thoroughly and approved by Microsoft, we can start with the review process.

P.S. a forked version of this code will be checked in to the Khronos Vulkan ICD Loader repository.

Thanks,
Ofir

lenny-lunarg pushed a commit to KhronosGroup/Vulkan-LoaderAndValidationLayers that referenced this pull request Aug 7, 2017
…y keys

This change extends the functionality of searching for ICD JSONs by adding
registry locations specific to given display adapter and software components
associated with this display adapter.

The exact locations in registry are queried using Windows public PnP
Configuration Manager API[1].

This change is required, as previous ICD locations (constant path
in "HKLM/Software") may be unreachable for drivers and their installers on
Windows RS3[2].

Similar change is being made for OpenCL[2]

[1]https://msdn.microsoft.com/en-us/library/windows/hardware/ff549713.aspx
[2]KhronosGroup/OpenCL-ICD-Loader#21

Change-Id: I815c2b5dca9d56e1486760b9e0082ccac40af502
@ofirc
Copy link
Contributor Author

ofirc commented Aug 9, 2017

The changes are ready to go.

lenny-lunarg pushed a commit to KhronosGroup/Vulkan-LoaderAndValidationLayers that referenced this pull request Aug 9, 2017
This change extends the functionality of searching for ICD JSONs by
adding registry locations specific to given display adapter and
software components associated with this display adapter.

The exact locations in registry are queried using Windows public PnP
Configuration Manager API[1].

This change is required, as previous ICD locations (constant path
in "HKLM/Software") may be unreachable for drivers and their
installers on Windows RS3[2].

Similar change is being made for OpenCL[2]

[1]https://msdn.microsoft.com/en-us/library/windows/hardware/ff549713.aspx
[2]KhronosGroup/OpenCL-ICD-Loader#21
lenny-lunarg pushed a commit to KhronosGroup/Vulkan-LoaderAndValidationLayers that referenced this pull request Aug 10, 2017
This change extends the functionality of searching for ICD JSONs by
adding registry locations specific to given display adapter and
software components associated with this display adapter.

The exact locations in registry are queried using Windows public PnP
Configuration Manager API[1].

This change is required, as previous ICD locations (constant path
in "HKLM/Software") may be unreachable for drivers and their
installers on Windows RS3[2].

Similar change is being made for OpenCL[2]

[1]https://msdn.microsoft.com/en-us/library/windows/hardware/ff549713.aspx
[2]KhronosGroup/OpenCL-ICD-Loader#21
lenny-lunarg pushed a commit to KhronosGroup/Vulkan-LoaderAndValidationLayers that referenced this pull request Aug 10, 2017
This change extends the functionality of searching for ICD JSONs by
adding registry locations specific to given display adapter and
software components associated with this display adapter.

The exact locations in registry are queried using Windows public PnP
Configuration Manager API[1].

This change is required, as previous ICD locations (constant path
in "HKLM/Software") may be unreachable for drivers and their
installers on Windows RS3[2].

Similar change is being made for OpenCL[2]

[1]https://msdn.microsoft.com/en-us/library/windows/hardware/ff549713.aspx
[2]KhronosGroup/OpenCL-ICD-Loader#21
@ofirc
Copy link
Contributor Author

ofirc commented Aug 23, 2017

@bashbaug @kepatil ?

#ifdef _WIN64
static const char OPENCL_REG_SUB_KEY[] = "OpenCLDriverName";
#else
static const char OPENCL_REG_SUB_KEY[] = "OpenCLDriverNameWow";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, WoW refers only to executing 32-bit code on 64-bit OS.

To preserve this semantics, it would be better to change this #ifdef _WIN64 to checking for WoW with IsWow64Process() Windows API function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true in general, but in this case it is aligned with how OpenGL registers its ICDs, and uses the Wow suffix to refer to the 32-bit version of its ICD.
Besides, _WIN64 is compile-time, IsWow64Process() is run-time, we don't want the performance overhead incurred by this call if not actually needed.

@kepatil
Copy link
Contributor

kepatil commented Sep 5, 2017

@ofirc Changes to ICD extension spec were approved at the last working group call. I will act on this pull request shortly.

@kepatil kepatil self-assigned this Sep 21, 2017
@b-sumner
Copy link

I understand this is ready to go. Kedar, would you please commit? Our own tests show this is working as expected.

@kepatil
Copy link
Contributor

kepatil commented Oct 3, 2017

Relevant updates to the ICD extension were accepted in KhronosGroup/OpenCL-Registry#30.

@kepatil kepatil merged commit 188be60 into KhronosGroup:master Oct 3, 2017
joekilner pushed a commit to joekilner/OpenCL-ICD-Loader that referenced this pull request Jun 20, 2020
Starting from Windows 10 RS3+ Graphics Drivers are no longer allowed to
write HKLM entries, and hence the current registry key:
HKLM\SOFTWARE\Khronos\OpenCL\Vendors

Is no longer applicable to graphics drivers for registering their ICD.
Instead, graphics drivers can write to either of the 2 registry keys
below:
1) Display Adapter HKR
2) Software Components HKR

Graphics Drivers (starting from RS3) have dedicated component INF files and are
referenced via the CopyINF directive and/or (optional) "INF AddComponent directive".

Every such component, e.g. OpenCL instance (ICD), is assigned a unique device
instance ID (e.g. 0000, 0001, etc) under the "Software Component" GUID and use it to
store the path to its ICD.

Therefore we enumerate ICDs in the following places (and also in the following order):
1) Display Adapters HKRs (using the display adapter GUID)
2) Software Component HKRs (using the CM_* APIs to traverse the device tree)
3) The usual "Vendors" registry key (see above)

Corresponding ICD extension updates were made with KhronosGroup/OpenCL-Registry#30.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants