[AMD Official Use Only - Internal Distribution Only]

Hi Das,

It seems more reasonable to put spin lock in ring structure, so that KIQ ring 
can be treated like other normal ring.
and do we have other paths to call kiq ring except for kernel initialization? 
it seems the kfd and kgd will use kiq in one thread..

thanks.

Best Regards,
Kevin
________________________________
From: amd-gfx <[email protected]> on behalf of Nirmoy Das 
<[email protected]>
Sent: Saturday, March 13, 2021 1:08 AM
To: Koenig, Christian <[email protected]>
Cc: Das, Nirmoy <[email protected]>; [email protected] 
<[email protected]>
Subject: [PATCH 1/1] drm/amdgpu: wrap kiq ring ops with kiq spinlock

KIQ ring is being operated by kfd as well as amdgpu.
KFD is using kiq lock, we should the same from amdgpu side
as well.

Signed-off-by: Nirmoy Das <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 1915b9b95106..892dc6b269fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -462,20 +462,25 @@ int amdgpu_gfx_disable_kcq(struct amdgpu_device *adev)
 {
         struct amdgpu_kiq *kiq = &adev->gfx.kiq;
         struct amdgpu_ring *kiq_ring = &kiq->ring;
-       int i;
+       int i, r;

         if (!kiq->pmf || !kiq->pmf->kiq_unmap_queues)
                 return -EINVAL;

+       spin_lock(&adev->gfx.kiq.ring_lock);
         if (amdgpu_ring_alloc(kiq_ring, kiq->pmf->unmap_queues_size *
-                                       adev->gfx.num_compute_rings))
+                                       adev->gfx.num_compute_rings)) {
+               spin_unlock(&adev->gfx.kiq.ring_lock);
                 return -ENOMEM;
+       }

         for (i = 0; i < adev->gfx.num_compute_rings; i++)
                 kiq->pmf->kiq_unmap_queues(kiq_ring, 
&adev->gfx.compute_ring[i],
                                            RESET_QUEUES, 0, 0);
+       r = amdgpu_ring_test_helper(kiq_ring);
+       spin_unlock(&adev->gfx.kiq.ring_lock);

-       return amdgpu_ring_test_helper(kiq_ring);
+       return r;
 }

 int amdgpu_queue_mask_bit_to_set_resource_bit(struct amdgpu_device *adev,
@@ -518,12 +523,13 @@ int amdgpu_gfx_enable_kcq(struct amdgpu_device *adev)

         DRM_INFO("kiq ring mec %d pipe %d q %d\n", kiq_ring->me, 
kiq_ring->pipe,
                                                         kiq_ring->queue);
-
+       spin_lock(&adev->gfx.kiq.ring_lock);
         r = amdgpu_ring_alloc(kiq_ring, kiq->pmf->map_queues_size *
                                         adev->gfx.num_compute_rings +
                                         kiq->pmf->set_resources_size);
         if (r) {
                 DRM_ERROR("Failed to lock KIQ (%d).\n", r);
+               spin_unlock(&adev->gfx.kiq.ring_lock);
                 return r;
         }

@@ -532,6 +538,7 @@ int amdgpu_gfx_enable_kcq(struct amdgpu_device *adev)
                 kiq->pmf->kiq_map_queues(kiq_ring, &adev->gfx.compute_ring[i]);

         r = amdgpu_ring_test_helper(kiq_ring);
+       spin_unlock(&adev->gfx.kiq.ring_lock);
         if (r)
                 DRM_ERROR("KCQ enable failed\n");

--
2.30.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7CKevin1.Wang%40amd.com%7C0a90c6383d634a5226a108d8e5797c08%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637511657234189238%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=sfa27Sz8wcIyHJqaE9f8aXQMeUxbyynpAONZHUYoogY%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to