[PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid
Felix Kuehling
felix.kuehling at amd.com
Mon Jan 6 16:04:50 UTC 2020
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.
>
> 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%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cfelix.kuehling%40amd.com%7C0ebb82d62d1044ea57b608d791f5b021%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637138356784407460&sdata=K8zhHT2YYFj8LXdD3YiihtNkbKNwa0ml6nQZ74zF828%3D&reserved=0
>
More information about the amd-gfx
mailing list