Remove libdrm_amdgpu and other unnecessary code #703

evelikov · 2022-03-06T19:18:34Z

This series removes the a bunch of dead or no longer needed code. In particular:

libdrm_amdgpu has two better alternatives in-tree - individual and the gpu_metrics sysfs nodes
- bonus now we properly track/open only the required ones
ifdef spaghetty in keybinds code
the find_folder() API - we can use ls() instead
excessive pipelines when fetching basic data

The vendor/device handling is still a bit iffy - I might be sorting that out next.

Note: this has been only lightly tested, so please give it a spin on your end and report issues.

There is no such file in-tree and never was. Signed-off-by: Emil Velikov <[email protected]>

Drop the ifdef and simplify the .cpp file. Signed-off-by: Emil Velikov <[email protected]>

Currently we pipeline a bunch of commands alike cat | grep | sed, there we can do all that job with a single sed invocation - use that. Signed-off-by: Emil Velikov <[email protected]>

The code was added back in 2020 and seemingly never built. Just drop it - if needed it can be git reverted at some point. Signed-off-by: Emil Velikov <[email protected]>

There are two alternatives - hwmon entries and gpu_metrics sysfs file. Signed-off-by: Emil Velikov <[email protected]>

Currently it can give you a regular file, block/char device, fifo or a socket. Signed-off-by: Emil Velikov <[email protected]>

Makes the code much easier to read. Signed-off-by: Emil Velikov <[email protected]>

Signed-off-by: Emil Velikov <[email protected]>

evelikov · 2022-03-12T11:38:00Z

Been using this for a few days without issues. Just some 600+ loc removed

@flightlessmango @jackun any takers?

Faalagorn · 2022-03-12T15:03:01Z

I gave it a quick try on my RX 580, so far no issues at the first glance, I'll keep on running with that to see if I encounter any issues down the road.

flightlessmango · 2022-03-12T15:44:55Z

It looks good to me, I would just like @jackun to sign off as well

src/overlay.cpp

jackun · 2022-03-12T16:40:16Z

Do Vega APUs support gpu_metrics (a.k.a why libdrm etc used in first place)? At least Vega64 dgpu doesn't seem to.
e: vega12, vega20, renoir, vangogh, navi10, navi21(?), MI cards, maybe more... so only APUs since 2020?

src/overlay.cpp

evelikov · 2022-03-12T18:07:38Z

Do Vega APUs support gpu_metrics (a.k.a why libdrm etc used in first place)? At least Vega64 dgpu doesn't seem to. e: vega12, vega20, renoir, vangogh, navi10, navi21(?), MI cards, maybe more... so only APUs since 2020?

AFAICT the standalone nodes - gpu_busy_percent, temp1_input and others, are present on all GPUs. So even if the composite gpu_metrics node is missing, all GPU will still have basic data.

That said, looks like I've forgot a check - currently the gpu_metrics node will be omitted if gpu_busy_percent is missing. It seems very unlikely to happen, but still not the right thing to do. Will add extra patch on top.

Can be trivially replaced with ls() v2: Move break where it's supposed to. Signed-off-by: Emil Velikov <[email protected]>

We have already enforced that a few lines above just after parsing the vendor node. Signed-off-by: Emil Velikov <[email protected]>

Currently we can get the load and temp stats either from the standalone nodes or from the gpu_metrics (binary) sysfs node. Fix the next to handle that. Signed-off-by: Emil Velikov <[email protected]>

Currently we open the standalone busy, temp, gpu/memory clock and power_usage nodes, even if gpu_metrics is present. At the same time, we correctly ignore them when doing the read-only. So just avoid opening them all together. Signed-off-by: Emil Velikov <[email protected]>

No longer applicable, since the libdrm path is gone and hwmon is required - either partially or in full. Signed-off-by: Emil Velikov <[email protected]>

jackun · 2022-03-12T18:12:29Z

IIRC gpu_busy_percent gave "Invalid Argument" on Vega 3 etc. unless patched since #53

evelikov · 2022-03-12T18:16:29Z

Sounds like something which should be reported and fixed in the amdgpu driver. Adding/maintaining a complete code-path for such a bug that does not sound palatable IMHO.

evelikov · 2022-03-12T19:55:21Z

~~Doing a quick test shows that something has gone off.~~
~~Quickly flipping to draft - so this doesn't go in and break the world~~

Fixed, see below

Above all, we really don't need the gpu_busy_percent node, if the GPU exposes a gpu_metrics node, Although looking closer, the gpu_busy_percent check is meant for something else - to distinguish between the card node (cardX) and the card output node (cardX-output-foo). To top it all up, the check at the very end implies that we can get a case where gpu_metrics and gpu_busy_perfect is missing ... that's not possible. So instead, drop the early gpu_busy_perfect check and properly mandate it later on. Signed-off-by: Emil Velikov <[email protected]>

evelikov · 2022-03-12T20:34:36Z

The gpu_busy_percent check that the last commit removed had a very misleading meaning - it was used to distinguish between card0 and card0-DP-1 nodes. See the commit description (and inline comment) for details.

jackun · 2022-03-12T20:48:55Z

to distinguish between the card node (cardX) and the
card output node (cardX-output-foo).

More of a is using amdgpu check but filters those (cardX-output-foo) out too :P

evelikov · 2022-03-15T16:42:29Z

Fwiw there's another dozen or so patches series sitting on top of this work - #707

jackun · 2022-03-30T09:34:47Z

APUs need newer (v)bioses, I guess: https://github.com/torvalds/linux/blob/d888c83fcec75194a8a48ccd283953bdba7b2550/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c#L1291

evelikov force-pushed the misc-mixes branch from 9b14969 to 90be72a Compare March 6, 2022 19:22

evelikov added 8 commits March 12, 2022 10:28

editorconfig: drop irrelevant Makefile section

1ed8815

There is no such file in-tree and never was. Signed-off-by: Emil Velikov <[email protected]>

keybinds: move ifdef spaghetti into the header

83ce3ac

Drop the ifdef and simplify the .cpp file. Signed-off-by: Emil Velikov <[email protected]>

Simplify cpu/mem/os/glxinfo parsing

7ad24b4

Currently we pipeline a bunch of commands alike cat | grep | sed, there we can do all that job with a single sed invocation - use that. Signed-off-by: Emil Velikov <[email protected]>

Remove unused X11/drm auth code

abbe523

The code was added back in 2020 and seemingly never built. Just drop it - if needed it can be git reverted at some point. Signed-off-by: Emil Velikov <[email protected]>

Remove libdrm_amdgpu metrics

68ce8f8

There are two alternatives - hwmon entries and gpu_metrics sysfs file. Signed-off-by: Emil Velikov <[email protected]>

List regular files with ls(.. LS_FILES)

ab684a2

Currently it can give you a regular file, block/char device, fifo or a socket. Signed-off-by: Emil Velikov <[email protected]>

Use a switch in ls() d_type handling

ffb9b59

Makes the code much easier to read. Signed-off-by: Emil Velikov <[email protected]>

Remove unused LS_FLAGS::LS_ALL

d75b4d9

Signed-off-by: Emil Velikov <[email protected]>

evelikov force-pushed the misc-mixes branch from 90be72a to 3552434 Compare March 12, 2022 14:52

jackun reviewed Mar 12, 2022

View reviewed changes

src/overlay.cpp Outdated Show resolved Hide resolved

jackun reviewed Mar 12, 2022

View reviewed changes

src/overlay.cpp Show resolved Hide resolved

evelikov added 5 commits March 12, 2022 18:10

Remove find_folder() API

2aa49ca

Can be trivially replaced with ls() v2: Move break where it's supposed to. Signed-off-by: Emil Velikov <[email protected]>

Remove dummy vendorID overwrite in init_gpu_stats()

a7e8ae9

We have already enforced that a few lines above just after parsing the vendor node. Signed-off-by: Emil Velikov <[email protected]>

Fix gpu_stats logic check

5143229

Currently we can get the load and temp stats either from the standalone nodes or from the gpu_metrics (binary) sysfs node. Fix the next to handle that. Signed-off-by: Emil Velikov <[email protected]>

overlay: completely remove force_amdgpu_hwmon option

9c6de02

No longer applicable, since the libdrm path is gone and hwmon is required - either partially or in full. Signed-off-by: Emil Velikov <[email protected]>

evelikov force-pushed the misc-mixes branch from 3552434 to 975535b Compare March 12, 2022 18:22

jackun approved these changes Mar 12, 2022

View reviewed changes

evelikov marked this pull request as draft March 12, 2022 19:54

evelikov force-pushed the misc-mixes branch from 975535b to 816d9f6 Compare March 12, 2022 20:31

evelikov marked this pull request as ready for review March 12, 2022 20:31

evelikov mentioned this pull request Mar 13, 2022

Various mangoapp fixes, et al #707

Merged

jackun merged commit 11142b5 into flightlessmango:master Mar 15, 2022

evelikov deleted the misc-mixes branch March 27, 2022 11:34

jackun mentioned this pull request Mar 4, 2023

Use libdrm_amdgpu as an alternative GPU load information source #925

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove libdrm_amdgpu and other unnecessary code #703

Remove libdrm_amdgpu and other unnecessary code #703

evelikov commented Mar 6, 2022

evelikov commented Mar 12, 2022

Faalagorn commented Mar 12, 2022

flightlessmango commented Mar 12, 2022

jackun commented Mar 12, 2022 •

edited

Loading

evelikov commented Mar 12, 2022

jackun commented Mar 12, 2022

evelikov commented Mar 12, 2022

evelikov commented Mar 12, 2022 •

edited

Loading

evelikov commented Mar 12, 2022

jackun commented Mar 12, 2022

evelikov commented Mar 15, 2022

jackun commented Mar 30, 2022

Remove libdrm_amdgpu and other unnecessary code #703

Remove libdrm_amdgpu and other unnecessary code #703

Conversation

evelikov commented Mar 6, 2022

evelikov commented Mar 12, 2022

Faalagorn commented Mar 12, 2022

flightlessmango commented Mar 12, 2022

jackun commented Mar 12, 2022 • edited Loading

evelikov commented Mar 12, 2022

jackun commented Mar 12, 2022

evelikov commented Mar 12, 2022

evelikov commented Mar 12, 2022 • edited Loading

evelikov commented Mar 12, 2022

jackun commented Mar 12, 2022

evelikov commented Mar 15, 2022

jackun commented Mar 30, 2022

jackun commented Mar 12, 2022 •

edited

Loading

evelikov commented Mar 12, 2022 •

edited

Loading