Skip to content

Comments

Don't access srbm when disabled#152

Merged
clbr merged 1 commit intoclbr:masterfrom
danielzgtg:fixDisablingSrbmRead
Feb 22, 2023
Merged

Don't access srbm when disabled#152
clbr merged 1 commit intoclbr:masterfrom
danielzgtg:fixDisablingSrbmRead

Conversation

@danielzgtg
Copy link
Contributor

@danielzgtg danielzgtg commented Feb 22, 2023

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 clbr merged commit ec97e6f into clbr:master Feb 22, 2023
@clbr
Copy link
Owner

clbr commented Feb 22, 2023

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

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.

2 participants