Hi Christian, On 9/3/2022 4:49 AM, Christian Gmeiner wrote:
I had a quick look at what vivantes kernel driver did. It uses a per gpu instance variable powerBaseAddress that gets set accordingly. I am not sure if I really like the gpu_fix_reg_address(..) idea, as it gets called on every register read and write. For me I see two other possible solutions: 1) Add two seperate helpers ala gpu_read_power() and gpu_write_power() where we do the if beast. 2) Add a power register offset variable to etnaviv_gpu and explicitly use it on for reads and writes - like the Vivante driver does. But that's just my personal opinion. Can't wait to hear what Lucas thinks.
Thanks for reviewing so fast! I honestly agree. It felt kind of dirty modifying gpu_write and gpu_read. The reason I went for it is I was thinking that in most cases the compiler is going to optimize the ugly "if" out. The two solutions listed above both sound good. They would need a special case in etnaviv_core_dump_registers, but that's probably much less nasty than modifying gpu_read and gpu_write. Any preferences from everyone on which of the other two options I should do? The gpu_read_power and gpu_write_power approach sounds pretty clean to me. Also, dropping David Airlie from the chain -- his email address bounced on my original patch messages. Doug
