[PATCH] drm/etnaviv: Read some FE registers twice

[Derek Foreman]

Hello,

On 2024-05-16 12:36, Lucas Stach wrote:
> Hi Derek,
>
> Am Freitag, dem 03.05.2024 um 14:11 -0500 schrieb Derek Foreman:
>> On some hardware (such at the GC7000 rev 6009), these registers need to be
>> read twice to return the correct value. Hide that in gpu_read().
>>
>> Signed-off-by: Derek Foreman <derek.foreman at collabora.com>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> index 197e0037732e..0f67c62be3d1 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> @@ -11,6 +11,7 @@
>>   #include "etnaviv_mmu.h"
>>   #include "etnaviv_drv.h"
>>   #include "common.xml.h"
>> +#include "state.xml.h"
>>   
>>   struct etnaviv_gem_submit;
>>   struct etnaviv_vram_mapping;
>> @@ -170,6 +171,13 @@ static inline void gpu_write(struct etnaviv_gpu *gpu, u32 reg, u32 data)
>>   
>>   static inline u32 gpu_read(struct etnaviv_gpu *gpu, u32 reg)
>>   {
>> +	/* On some variants, such as the GC7000 6009, some FE registers
>                                           GC7000 rev 6009
>> +	 * need two reads to be consistent. Do that extra read here and
>> +	 * throw away the result.
>> +	 */
> Please use the common comment style of this driver with a blank line
> after the /*
Oops - done for v2. Thanks.
>> +	if (reg >= VIVS_FE_DMA_STATUS && reg <= VIVS_FE_AUTO_FLUSH)
>> +		readl(gpu->mmio + reg);
> I don't think it matters much, but we can save some of the overhead
> here by using readl_relaxed.

Can I just not do this? I'm concerned that losing the memory barrier 
could result in weird reads again. The galcore driver doesn't use a 
relaxed read, and these registers are only ever read when something goes 
wrong - so I don't think performance is a concern at all?

Thanks,
Derek

> Regards,
> Lucas
>
>> +
>>   	return readl(gpu->mmio + reg);
>>   }
>>