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

fix: Can't detect librocm 6.0.x #761

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

imwints
Copy link
Contributor

@imwints imwints commented Feb 12, 2024

librocm reports it's version as 7.0.0.0 in the 6.0.x release series.

Closes: #759

@aristocratos
Copy link
Owner

aristocratos commented Feb 12, 2024

@imwints
Well, that's not confusing at all...

But are you sure the minor version won't change until version 7?

Wouldn't it make sense that version 7 would be 8.0.0.0 internally?

@aristocratos
Copy link
Owner

Why is that not confusing?

That was irony...

But I was asking why you are checking version.minor == 0, we are only concerned with a major release change, since AMD hopefully won't do any breaking changes until next major release.

And by the current logic that would be 8.0.0.0 (or 9.0.0.0 if they are skipping one again).

@imwints
Copy link
Contributor Author

imwints commented Feb 12, 2024

:) went over my head

And by the current logic that would be 8.0.0.0 (or 9.0.0.0 if they are skipping one again).

I just thought if they change their scheme again and release a 7.x with the version 7.1.0.0 or similar this is save for now and would only accept the real 6.x for now. I just wanted to limit the allowed version range so this just wouldn't allow any new release. But I'm happy to remove that

librocm reports it's version as 7.0.0.0 in the 6.0.x release series.
@aristocratos
Copy link
Owner

Hm, there is also a rsmi_version_str_get() function that returns a string.

Descriptions:

rsmi_version_get(): Get the build version information for the currently running build of RSMI

rsmi_version_str_get(): Get the driver version string for the current system.

Might be an idea to test rsmi_version_str_get() instead and check if it's correct then if so check if first char is 5 or 6.

@imwints Do you have a AMD card to test with?

The definition is at: https://rocm.docs.amd.com/projects/rocm_smi_lib/en/docs-6.0.0/.doxygen/docBin/html/group__VersQuer.html#ga6d4f034afcef2e50b4e8392c5dfd7a57

@imwints
Copy link
Contributor Author

imwints commented Feb 12, 2024

This will just give you the output of uname -r

@alerque
Copy link

alerque commented Feb 12, 2024

c.f. Upstream issue filed by @imwints.

@aristocratos
Copy link
Owner

This will just give you the output of uname -r

Huh, rsmi_version_str_get() gives you the output of uname -r?

That makes even less sense to me...

@imwints
Copy link
Contributor Author

imwints commented Feb 12, 2024

@aristocratos
Copy link
Owner

Will merge and create a new release later today, since I'm guessing it's unlikely anything will come from the issue in the ROCm repository in any reasonable time.

aristocratos
aristocratos previously approved these changes Feb 12, 2024
@aristocratos aristocratos merged commit 7a058e4 into aristocratos:main Feb 12, 2024
45 checks passed
@imwints imwints deleted the fix-rocm-5-6-supported branch February 20, 2024 10:26
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.

[BUG] btop from arch repos not reading AMD GPU dispite having rocm-smi-lib installed.
3 participants