The amdgpu driver has duplicated files for different versions of things, so it'll have thing_v6.c and thing_v7.c and thing_v8.c with a lot of duplicated functions.
The more common way of doing something like this would be to have structs of function pointers that get populated based on what version of GPU you have. You have one file with all the common functions that they can share, in the definitions for each GPU version you set the majority of the function pointers to the common version they all share and for ones that have to be different, you set them to their unique version. That way you can define all the common functions once, and point to them in the structs for each version.
Having a quick flick through the code now, they do use structs of function pointers in each version for common operations but they still don't abstract out the ones that are either identical or have very few differences that you could special case.
Refactoring such a giant driver for no performance gain is going to be extremely low on AMD's todo list, so it'll probably stay like that. It just doesn't look like anything else in the kernel
This is literally what what everyone does in embedded C land. The repetitive definitions ate generally intended to be used with macros and are typically generated from the same definitions as the chip registers itself. Some places also auto generate embedded c/c++ structs or classes which imo is better. But I have gotten quite a bit of pushback for doing it.
A big issue also is the use of bitfields as much as reg duplication. Bitfields in c/c++ are a minefield if you don't lock down a known-good compiler version because there's just so much of it that's technically unspecified. Oftentimes you'll also have issues where certain register fields exist for some registers of a series and not the next or where the functionality/sizing/interpretation is context dependent or where certain locks or write orders are needed for correct access and these are often handed with presence checking macros.
IMO, if we want better driver code, it's time for GCC/Clang to nail down the bitfield layouts for the embedded use cases. This has been broken for far too long.
It would be very difficult to get accepted. You'd have to get the AMDGPU driver maintainers on board, and you'd probably have to do a lot of it at once to justify the change. It would also take some discussion, and you're talking about refactoring a lot of stuff which probably moves underneath you during this, so you have to keep iterating to keep up with the changes, all without knowing if they'll even end up taking it...
Changes like this are probably a good way to get started but I would guess the AMDGPU driver is one of the worst places to get started as a beginner.
I mean, each new version is separate, correct? So the only change that can happen under you is when something is backported. How often does that happen for a gpu driver, and how far back does that go?
The amdgpu driver has duplicated files for different versions of things, so it'll have thing_v6.c and thing_v7.c and thing_v8.c with a lot of duplicated functions.
The more common way of doing something like this would be to have structs of function pointers that get populated based on what version of GPU you have. You have one file with all the common functions that they can share, in the definitions for each GPU version you set the majority of the function pointers to the common version they all share and for ones that have to be different, you set them to their unique version. That way you can define all the common functions once, and point to them in the structs for each version.
Having a quick flick through the code now, they do use structs of function pointers in each version for common operations but they still don't abstract out the ones that are either identical or have very few differences that you could special case.
Refactoring such a giant driver for no performance gain is going to be extremely low on AMD's todo list, so it'll probably stay like that. It just doesn't look like anything else in the kernel