[PATCH 1/1] drm/amdgpu: Flush TLB after mapping for VG20+XGMI

[Paul Menzel]

Dear Philip,


Thank you for your response.

Am 04.04.22 um 18:52 schrieb philip yang:

> On 2022-04-04 12:19, Paul Menzel wrote:

>> Am 01.04.22 um 21:57 schrieb Philip Yang:
>>> For VG20 + XGMI bridge, all mappings PTEs cache in TC, this may have
>>> stall invalid PTEs in TC because one cache line has 8 pages. Need always
>>
>> Can you please rephrase. “may have stall …” is really hard to understand.
> The patch already pushed and merged.

Sorry, but that really sucks, that commits with such a hard to 
understand commit message are merged.

>>> flush_tlb after updating mapping.
>>
>> Maybe:
>>
>> So, always flush_tlb after updating the mapping.
>>
>>> Signed-off-by: Philip Yang <Philip.Yang at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index f0aec04111a3..687c9a140645 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -837,6 +837,12 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, 
>>> struct amdgpu_vm *vm,
>>>           goto error_unlock;
>>>       }
>>>   +    /* Vega20+XGMI where PTEs get inadvertently cached in L2 texture cache,
>>> +     * heavy-weight flush TLB unconditionally.
>>> +     */
>>> +    flush_tlb |= (adev->gmc.xgmi.num_physical_nodes &&
>>> +              adev->ip_versions[GC_HWIP][0] == IP_VERSION(9, 4, 0));
>>> +
>>>       memset(&params, 0, sizeof(params));
>>>       params.adev = adev;
>>>       params.vm = vm;
>>
>> Did you do any performance measurement, if that flushing affects anything?
> 
> There was another patch to optimize TLB flush to improve map to GPU performance,
> for this config, always flush TLB after updating mapping is the original
> performance before the optimization.

So, why didn’t you reference this commit in the commit message, and also 
did not give that information in the commit message?


Kind regards,

Paul