Hi Doug, Am Montag, dem 05.09.2022 um 14:05 -0700 schrieb Doug Brown: > 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. Yes, please add a specialized wrapper for the power register accesses. Regards, Lucas
