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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
Sent: Monday, January 6, 2020 10:23 AM
To: Kuehling, Felix <Felix.Kuehling@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Sierra Guiza, Alejandro (Alex) <Alex.Sierra@xxxxxxx>
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@xxxxxxx>
---
   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@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7C
felix.kuehling%40amd.com%7C0ebb82d62d1044ea57b608d791f5b021%7C3dd8961
fe4884e608e11a82d994e183d%7C0%7C0%7C637138356784407460&amp;sdata=K8zh
HT2YYFj8LXdD3YiihtNkbKNwa0ml6nQZ74zF828%3D&amp;reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux