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.

Sign up for GitHub

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

Jump to bottom

Improve GPU debug reports #7456

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Improve GPU debug reports #7456

wants to merge 5 commits into from

Conversation

System64fumo
Copy link
Contributor

Describe your PR, what does it fix/add?

Instead of using weird "hacks" to get GPU information, Just get whatever libdrm reports.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

At the moment this uses /dev/dri/card0.
This implementation is not as verbose as before, Card model is not shown, unaware of a way to show it.

Is it ready for merging, or does it need work?

I'd say it's ready i guess.

@System64fumo
Improve GPU debug reports
3bc7601 
Instead of using weird "hacks" to get GPU information, Just get whatever libdrm reports.
This is by no means perfect and omits specific GPU details such as the model.
@MahouShoujoMivutilde
Copy link
Contributor

MahouShoujoMivutilde commented Aug 21, 2024 •
edited
Loading

More GPU info can be obtained like so https://gitlab.freedesktop.org/mesa/demos/-/blob/main/src/demos/glinfo.c?ref_type=heads#L23-25

GL_VERSION: 4.6.0 NVIDIA 560.31.02
GL_RENDERER: NVIDIA GeForce GTX 970/PCIe/SSE2
GL_VENDOR: NVIDIA Corporation
GL_EXTENSIONS: blah blah blah long string
GL_SHADING_LANGUAGE_VERSION = 4.60 NVIDIA
GLU_VERSION: 1.3
GLU_EXTENSIONS: GLU_EXT_nurbs_tessellator GLU_EXT_object_space_tess 
GLUT_API_VERSION: 4
GLUT_XLIB_IMPLEMENTATION: 13

You also need to run drmFreeVersion() after you done with drmGetVersion().

https://github.com/swaywm/sway/blob/7288f77bbe275825a0e0b011db873d9367782af0/sway/server.c#L148-L175

@System64fumo
Copy link
Contributor Author

Yup sorry, Completely forgot to include that, As for additional GPU info vaxry said the current output is fine.
At the end of the day this is not a fetch program, Specifics shouldn't matter i guess.

This is just an excuse, I'm too lazy to add more crud

@vaxerski
Copy link
Member

y no clang-format

@System64fumo
Copy link
Contributor Author

Didn't know that was a thing?

vaxerski
vaxerski reviewed Aug 23, 2024
View reviewed changes
src/debug/HyprCtl.cpp
const std::string GPUINFO = execAndGet("lspci -vnn | grep VGA");
#endif
result += "GPU information: \n" + GPUINFO;
int fd = open("/dev/dri/card0", O_RDONLY | O_CLOEXEC);
Copy link
Member

Choose a reason for hiding this comment

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

this is dumb, we may not even be running on card0. You should get the card(s) path(s) from the current aq session g_pCompositor->m_pAqBackend->session

src/debug/HyprCtl.cpp Outdated
result += "GPU information: \n" + GPUINFO;
int fd = open("/dev/dri/card0", O_RDONLY | O_CLOEXEC);
if (fd < 0) {
throw std::runtime_error("Failed to open /dev/dri/card0");
Copy link
Member

Choose a reason for hiding this comment

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

??? just skip this log

src/debug/HyprCtl.cpp Outdated
drmVersion* version = drmGetVersion(fd);
if (!version) {
close(fd);
throw std::runtime_error("Failed to get DRM version");
Copy link
Member

Choose a reason for hiding this comment

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

again

src/helpers/MiscFunctions.cpp
#else
const std::string GPUINFO = execAndGet("lspci -vnn | grep VGA");
#endif
int fd = open("/dev/dri/card0", O_RDONLY | O_CLOEXEC);
Copy link
Member

Choose a reason for hiding this comment

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

same stuff here, also why are you duping code? make this into a function? renderer does the same, btw, idk why this is needed here

@System64fumo
Copy link
Contributor Author

No clue how to use "g_pCompositor->m_pAqBackend->session" but hopefully this is better.

@MahouShoujoMivutilde
Copy link
Contributor

MahouShoujoMivutilde commented Aug 23, 2024 •
edited
Loading

Probably like so

Hyprland/src/render/Renderer.cpp

Line 38 in 688fe5c

for (auto& dev : g_pCompositor->m_pAqBackend->session->sessionDevices) {

Btw, this

Hyprland/src/render/Renderer.cpp

Line 48 in 688fe5c

Debug::log(LOG, "DRM driver information: {} v{}.{}.{} from {} description {}", name, DRMV->version_major, DRMV->version_minor, DRMV->version_patchlevel,

is useless, at least on nvidia:

[LOG] DRM driver information: nvidia-drm v0.0.0 from 20160202 description NVIDIA DRM driver

Well, it logged elsewhere with cat /proc/driver/nvidia/version, so doesn't matter much.

But it's so weird how the function to get version doesn't actually return useful version number. Nvidia moment.

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

@vaxerski vaxerski vaxerski left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
core debug helpers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@System64fumo @MahouShoujoMivutilde @vaxerski