Re: [PATCH 2/2] drm/etnaviv: fix power register offset on GC300

[Doug Brown <doug@schmorgal.com>]

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