clbr / radeontop

GNU General Public License v3.0
810 stars 72 forks source link

add navi31 pci id #165

Open wgottwalt opened 6 months ago

wgottwalt commented 6 months ago

adds PCI id for NAVI31 (several Radeon RX 7900 XT, XTX and M)

clbr commented 6 months ago

Please see the reply in #159.

wgottwalt commented 6 months ago

Your approach using this script is a bit naive. The AMDGPU driver is half PCI and half platform driver. The platform part initializes the driver using the Radeon ATOM bios and if available the PSP (platform security processor since the first Ryzen gen). These parts don't require PCI ids from the driver, they get all information from the binary blobs. This is the case since NAVI2x. In short: Your script will never work again and you need to change how this part is done in radeontop, like manually adding PCI ids - what I did - or scan the PCI id database provided by libpciaccess, or better, do detecting via libpciaccess entirely.

clbr commented 6 months ago

In short: Your script will never work again

I know? Looks like you didn't check the existing issues and PRs.

you need to change how this part is done in radeontop

Nope. I don't much care about cards 10 years newer than what I have.

wgottwalt commented 6 months ago

Yeah, you are right, I didn't check. Didn't thought I have to put so much time in it.

That is actually a pity. The other option is amdgpu_top, but having a 13+ mb binary for such a tool is, uhm, lets call it insane. Though, thx for the honesty, it is so much better than coming up with some odd excuse. Hehe, it is actually funny, would have said the same in this situation. XD

clbr commented 6 months ago

If you want to make a fork with manually added pci ids, I'll link to it in the readme. But it's not something I want to do for cards I don't have.

wgottwalt commented 6 months ago

I was thinking about some more practical, like dropping this list entirely and switch to libpciaccess. Not sure yet if I will add this to radeontop or write a new tool for this.