[PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid
Christian König
ckoenig.leichtzumerken at gmail.com
Tue Jan 7 12:37:43 UTC 2020
Am 07.01.20 um 02:09 schrieb Sierra Guiza, Alejandro (Alex):
> [AMD Official Use Only - Internal Distribution Only]
>
>
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Monday, January 6, 2020 10:23 AM
> To: Kuehling, Felix <Felix.Kuehling at amd.com>; amd-gfx at lists.freedesktop.org; Sierra Guiza, Alejandro (Alex) <Alex.Sierra at amd.com>
> Subject: Re: [PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid
>
> Am 06.01.20 um 17:04 schrieb Felix Kuehling:
>> On 2020-01-05 10:41 a.m., Christian König wrote:
>>> Am 20.12.19 um 07:24 schrieb Alex Sierra:
>>>> This can be used directly from amdgpu and amdkfd to invalidate TLB
>>>> through pasid.
>>>> It supports gmc v7, v8, v9 and v10.
>>>>
>>>> Change-Id: I6563a8eba2e42d1a67fa2547156c20da41d1e490
>>>> Signed-off-by: Alex Sierra <alex.sierra at amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 6 ++
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 81
>>>> ++++++++++++++++++++++++
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 33 ++++++++++
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 34 ++++++++++
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 84
>>>> +++++++++++++++++++++++++
>>>> 5 files changed, 238 insertions(+)
>> [snip]
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index fa025ceeea0f..eb1e64bd56ed 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -38,10 +38,12 @@
>>>> #include "dce/dce_12_0_sh_mask.h"
>>>> #include "vega10_enum.h"
>>>> #include "mmhub/mmhub_1_0_offset.h"
>>>> +#include "athub/athub_1_0_sh_mask.h"
>>>> #include "athub/athub_1_0_offset.h"
>>>> #include "oss/osssys_4_0_offset.h"
>>>> #include "soc15.h"
>>>> +#include "soc15d.h"
>>>> #include "soc15_common.h"
>>>> #include "umc/umc_6_0_sh_mask.h"
>>>> @@ -434,6 +436,47 @@ static bool
>>>> gmc_v9_0_use_invalidate_semaphore(struct amdgpu_device *adev,
>>>> adev->pdev->device == 0x15d8)));
>>>> }
>>>> +static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct
>>>> amdgpu_device *adev,
>>>> + uint8_t vmid, uint16_t *p_pasid) {
>>>> + uint32_t value;
>>>> +
>>>> + value = RREG32(SOC15_REG_OFFSET(ATHUB, 0,
>>>> mmATC_VMID0_PASID_MAPPING)
>>>> + + vmid);
>>>> + *p_pasid = value & ATC_VMID0_PASID_MAPPING__PASID_MASK;
>>>> +
>>>> + return !!(value & ATC_VMID0_PASID_MAPPING__VALID_MASK);
>>>> +}
>>> Probably better to expose just this function in the GMC interface.
>> This is called below in gmc_v9_0_flush_gpu_tlb_pasid in the case that
>> the KIQ is not ready. It is not needed outside this file, so no need
>> to expose it in the GMC interface.
>>
>>
>>>> +
>>>> +static int gmc_v9_0_invalidate_tlbs_with_kiq(struct amdgpu_device
>>>> *adev,
>>>> + uint16_t pasid, uint32_t flush_type,
>>>> + bool all_hub)
>>>> +{
>>>> + signed long r;
>>>> + uint32_t seq;
>>>> + struct amdgpu_ring *ring = &adev->gfx.kiq.ring;
>>>> +
>>>> + spin_lock(&adev->gfx.kiq.ring_lock);
>>>> + amdgpu_ring_alloc(ring, 12); /* fence + invalidate_tlbs
>>>> +package*/
>>>> + amdgpu_ring_write(ring, PACKET3(PACKET3_INVALIDATE_TLBS, 0));
>>>> + amdgpu_ring_write(ring,
>>>> + PACKET3_INVALIDATE_TLBS_DST_SEL(1) |
>>>> + PACKET3_INVALIDATE_TLBS_ALL_HUB(all_hub) |
>>>> + PACKET3_INVALIDATE_TLBS_PASID(pasid) |
>>>> + PACKET3_INVALIDATE_TLBS_FLUSH_TYPE(flush_type));
>>>> That stuff is completely unrelated to the GMC and shouldn't be added
>>>> here.
>>> Where else should it go? To me TLB flushing is very much something
>>> that belongs in this file.
>>>
>>> Maybe you'd rather add "flush_tlbs_with_kiq" to amdgpu_ring_funcs and
>>> implement it in the gfx_v*.c GFX-IP code? I'm not sure that makes much
>>> sense either because it's only available on the KIQ ring.
>> Yes, something like this. We should probably add a amdgpu_kiq_funcs and expose the functions needed by the GMC code there.
>> See the amdgpu_virt_kiq_reg_write_reg_wait() case for reference, it is very similar and should probably be added to that function table as well.
>> Christian.
> To summarize:
> 1.- The idea is to add a new function pointer to the amdgpu_ring_funcs to flush the tlbs with kiq.
I would add a new amdgpu_kiq_funcs structure for that.
> 2.- This function pointer should be pointed and implemented on each of the gfx_v*.c under the gfx_*_ring_funcs_kiq struct. If this is true, Im not seeing this struct on the gfx_v10.c file.
> 3.- Use the amdgpu_virt_kiq_reg_write_reg_wait as a reference for the implementation.
Well yes and no, the amdgpu_virt_kiq_reg_write_reg_wait() was just an
example of a similar case.
The function amdgpu_virt_kiq_reg_write_reg_wait() should probably be
cleaned up and moved into the gfx_*.c files as well.
Regards,
Christian.
>
>>>
>>>> Christian.
>>>>
>>>> + amdgpu_fence_emit_polling(ring, &seq);
>>>> + amdgpu_ring_commit(ring);
>>>> + spin_unlock(&adev->gfx.kiq.ring_lock);
>>>> +
>>>> + r = amdgpu_fence_wait_polling(ring, seq, adev->usec_timeout);
>>>> + if (r < 1) {
>>>> + DRM_ERROR("wait for kiq fence error: %ld.\n", r);
>>>> + return -ETIME;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> /*
>>>> * GART
>>>> * VMID 0 is the physical GPU addresses as used by the kernel.
>>>> @@ -532,6 +575,46 @@ static void gmc_v9_0_flush_gpu_tlb(struct
>>>> amdgpu_device *adev, uint32_t vmid,
>>>> DRM_ERROR("Timeout waiting for VM flush ACK!\n");
>>>> }
>>>> +/**
>>>> + * gmc_v9_0_flush_gpu_tlb_pasid - tlb flush via pasid
>>>> + *
>>>> + * @adev: amdgpu_device pointer
>>>> + * @pasid: pasid to be flush
>>>> + *
>>>> + * Flush the TLB for the requested pasid.
>>>> + */
>>>> +static int gmc_v9_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>>>> + uint16_t pasid, uint32_t flush_type,
>>>> + bool all_hub)
>> Christian, do you agree that this function belongs in the GMC
>> interface? If not here, where do you suggest moving it?
>>
>> Regards,
>> Felix
>>
>>
>>>> +{
>>>> + int vmid, i;
>>>> + uint16_t queried_pasid;
>>>> + bool ret;
>>>> + struct amdgpu_ring *ring = &adev->gfx.kiq.ring;
>>>> +
>>>> + if (adev->in_gpu_reset)
>>>> + return -EIO;
>>>> +
>>>> + if (ring->sched.ready)
>>>> + return gmc_v9_0_invalidate_tlbs_with_kiq(adev,
>>>> + pasid, flush_type, all_hub);
>>>> +
>>>> + for (vmid = 1; vmid < 16; vmid++) {
>>>> +
>>>> + ret = gmc_v9_0_get_atc_vmid_pasid_mapping_info(adev, vmid,
>>>> + &queried_pasid);
>>>> + if (ret && queried_pasid == pasid) {
>>>> + for (i = 0; i < adev->num_vmhubs; i++)
>>>> + amdgpu_gmc_flush_gpu_tlb(adev, vmid,
>>>> + i, flush_type);
>>>> + break;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> +}
>>>> +
>>>> static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring
>>>> *ring,
>>>> unsigned vmid, uint64_t pd_addr)
>>>> {
>>>> @@ -693,6 +776,7 @@ static void gmc_v9_0_get_vm_pte(struct
>>>> amdgpu_device *adev,
>>>> static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>>>> .flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>>>> + .flush_gpu_tlb_pasid = gmc_v9_0_flush_gpu_tlb_pasid,
>>>> .emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>>>> .emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>>>> .map_mtype = gmc_v9_0_map_mtype,
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>>> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7C
>>> felix.kuehling%40amd.com%7C0ebb82d62d1044ea57b608d791f5b021%7C3dd8961
>>> fe4884e608e11a82d994e183d%7C0%7C0%7C637138356784407460&sdata=K8zh
>>> HT2YYFj8LXdD3YiihtNkbKNwa0ml6nQZ74zF828%3D&reserved=0
>>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list