[PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid

Christian König christian.koenig at amd.com
Mon Jan 6 16:23:29 UTC 2020


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.

>
>
>>
>> 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