clbr / radeontop

GNU General Public License v3.0
785 stars 69 forks source link

Don't access srbm when disabled #152

Closed danielzgtg closed 1 year ago

danielzgtg commented 1 year ago

This fixes a bug I introduced with #140.

Other GPUs such as laptop GPUs and older/newer GPUs have a different memory layout. I was told to add a check to disable the video encode/decode detection feature on those GPUs, but I accidentally only added a check during display. I missed the if statements around the actual memory reads. This PR adds these missing conditionals.

It is weird that a read (strange but I suppose it could happen when we get to hardware and might be expected from my experience of AMD drivers being unstable) is causing the problems, especially in userspace. There is also code doing mmap of these registers that ideally should be wrapped in the same condition, but it's architecturally complicated because initbits(int) is called after init_pci and there are a lot of data dependencies, so that code isn't changed. Also, I was originally planning to suggest the alternative solution of disabling the bits on laptop APUs, but that wouldn't have worked without the PR. Thanks to @madushan1000 in #149 for leading me to this solution by mentioning that his GPU is a model that the original if statement already excludes and that commenting it out did nothing, which made me look elsewhere and find this solution.

clbr commented 1 year ago

mmap does not do a read, so it shouldn't cause the same.