[PATCH] drm/amdgpu: Use uncached ioremap() for LoongArch

[Christian König]

Am 06.03.23 um 04:01 schrieb WANG Xuerui:
> On 2023/3/6 10:49, Huacai Chen wrote:
>> Hi, Christian,
>>
>> On Mon, Mar 6, 2023 at 12:40 AM Christian König
>> <christian.koenig at amd.com> wrote:
>>>
>>> Am 05.03.23 um 06:21 schrieb Huacai Chen:
>>>> LoongArch maintains cache coherency in hardware, but its WUC attribute
>>>> (Weak-ordered UnCached, which is similar to WC) is out of the scope of
>>>> cache coherency machanism. This means WUC can only used for write-only
>>>> memory regions. So use uncached ioremap() for LoongArch in the amdgpu
>>>> driver.
>>>
>>> Well NAK. This is leaking platform dependencies into the driver code.
>> Then is it acceptable to let ioremap() depend on 
>> drm_arch_can_wc_memory()?
>
> Note: he's likely meaning "is it acceptable to use 
> drm_arch_can_wc_memory() to decide between ioremap() and ioremap_wc()".
>
> Although I doubt it's acceptable to you (driver) folks either, because 
> while drm_arch_can_wc_memory() does isolate platform details from 
> driver proper, it's still papering over platform PCIe violation in 
> VRAM domain. Still better than having platform defines though.

Well agree on the PCIe violations, but drm_arch_can_wc_memory() is just 
for a completely different use case.

drm_arch_can_wc_memory() checks if system memory can be accessed write 
combined as well (which is a PCIe extension) or needs to be accessed 
with caching enabled (which is a core PCIe requirement).

So completely different topic and no using this here is not acceptable 
either.

The key point is that when WUC only works with write only mappings you 
*can't* use that to implement ioremap_wc().

> Also making use of drm_arch_can_wc_memory might fix this fdo issue [1] 
> on aarch64 too (where I replied earlier). It seems people simply can't 
> stop inventing such micro-architectures sadly...

I don't think that will help for this bug. WC on iomem is known to work 
correctly on aarch64 and well tested. What doesn't work is using WC on 
system memory.

And in the case of aarch64 it's not a core issue with the platform, but 
rather that some hw implements get it right and some get it wrong.

I already had an in deep discussion with ARM folks about that and it 
seems that some hw implementations think they can combine the core IP 
with some cheap PCIe root complex and it just magically works.

Regards,
Christian.

>
> [1]: https://gitlab.freedesktop.org/drm/amd/-/issues/2313
>
>>>
>>> When you have a limitation that ioremap_wc() can't guarantee read/write
>>> ordering then that's pretty clearly a platform bug and you would 
>>> need to
>>> apply this workaround to all drivers using ioremap_wc() which isn't
>>> really feasible.
>>>
>
> I agree in this case perhaps all of ioremap_wc() usages would have to 
> degrade into ioremap() for correctness on such platforms. In which 
> case amdgpu wouldn't have to be individually called out / touched 
> anyway. Whether this is easily doable/upstreamable is another question 
> though...
>