Skip to content

Conversation

@sparkleholic
Copy link
Contributor

Add the option GGML_VULKAN_V1_2_162 to make GGML
compatible with vulkan v1.2.162.
This option is OFF by default.

vulkan-header: v1.2.162
https://github.com/KhronosGroup/Vulkan-Headers/tree/v1.2.162

Add the option GGML_VULKAN_V1_2_162 to make GGML
compatible with vulkan v1.2.162.
This option is OFF by default.

vulkan-header: v1.2.162
https://github.com/KhronosGroup/Vulkan-Headers/tree/v1.2.162
@github-actions github-actions bot added build Compilation issues Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Nov 22, 2024
@0cc4m 0cc4m self-requested a review November 22, 2024 08:18
@0cc4m
Copy link
Collaborator

0cc4m commented Nov 22, 2024

v1.2.162 is very old, can you describe why you need it?

@sparkleholic
Copy link
Contributor Author

sparkleholic commented Nov 22, 2024

v1.2.162 is very old, can you describe why you need it?

In the consumer electronics industry, some mid-range SoCs still use that version.

@0cc4m
Copy link
Collaborator

0cc4m commented Nov 22, 2024

v1.2.162 is very old, can you describe why you need it?

In the consumer electronics industry, some mid-range SoCs still use that version.

And on those devices Vulkan makes a positive difference? I assume you are talking about phones and Android?

@sparkleholic
Copy link
Contributor Author

sparkleholic commented Nov 22, 2024

v1.2.162 is very old, can you describe why you need it?

In the consumer electronics industry, some mid-range SoCs still use that version.

And on those devices Vulkan makes a positive difference? I assume you are talking about phones and Android?

Yes, right. The cpu and gpu might be very different performance results. Not only the phones, home appliances could be applied as well. And android is just one of various platforms. There are various platforms applied to the electronics.
And like this case, the version of vulkan depends on the gpu drivers which provided by SoC providers.

@0cc4m
Copy link
Collaborator

0cc4m commented Nov 22, 2024

I'm just asking cause using the Vulkan backend on Android faces quite a few challenges that so far nobody has been willing to fix. See for example #10406

I'm not opposed to supporting an old API version, I'm just wondering if you have a device that runs well with this change or if this change only makes the library build on this device, and running it doesn't work yet.

@sparkleholic
Copy link
Contributor Author

I'm just asking cause using the Vulkan backend on Android faces quite a few challenges that so far nobody has been willing to fix. See for example #10406

I'm not opposed to supporting an old API version, I'm just wondering if you have a device that runs well with this change or if this change only makes the library build on this device, and running it doesn't work yet.

Yes, I understand.
Actually, this is just a patch for compilation issues with vukan v1.2.162, and even after applying this, there is still a problem in Qcom adreno 740 (adreno200 gpu driver), so I have requested Qcom to review this part. I will let you know here in a comment when the gpu driver review for this part is complete.

@jeffbolznv
Copy link
Collaborator

And like this case, the version of vulkan depends on the gpu drivers which provided by SoC providers.

The Vulkan headers are backward compatible and you can build with a newer header and run it on an older driver. Did you choose 1.2.162 because that's the Vulkan version a driver is reporting, or because this is the version of the header distributed with the Android NDK or something?

If there are build configurations that can't pick up a newer Vulkan SDK easily, then IMO we need to change the build to pull from https://github.com/KhronosGroup/Vulkan-Headers because #10206 will need it.

@sparkleholic
Copy link
Contributor Author

sparkleholic commented Nov 23, 2024

And like this case, the version of vulkan depends on the gpu drivers which provided by SoC providers.

The Vulkan headers are backward compatible and you can build with a newer header and run it on an older driver. Did you choose 1.2.162 because that's the Vulkan version a driver is reporting, or because this is the version of the header distributed with the Android NDK or something?

If there are build configurations that can't pick up a newer Vulkan SDK easily, then IMO we need to change the build to pull from https://github.com/KhronosGroup/Vulkan-Headers because #10206 will need it.

Thanks for the comment.
Yes, you're correct. vulkan-headers & loader are backward compatible, so newer version of those should work with older vulkan drivers. However it has some problems to apply a newer vulkan-loader as you mentioned due to build configuration, I'm digging the issue.

Looking at your #10206 code, it looks like you've already done a great job of branching for vulkan1.2 and 1.3, so I agree to make the necessary changes. For now, I'll just use my patch for communication purposes with other BSP engineers.

@sparkleholic
Copy link
Contributor Author

@0cc4m , @jeffbolznv
I'm going to close this ticket because ideally older drivers should be working with newer vulkan-headers & loader.
Hence, this patch seems to be necessary only in the case of that the build-configuration has a problem to update vulkan-headers & loader. Thanks for the comments and reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Compilation issues ggml changes relating to the ggml tensor library for machine learning Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants