Re: [PATCH 1/1] drm/amdgpu: wrap kiq ring ops with kiq spinlock

[Nirmoy <nirmodas@xxxxxxx>]

On 3/15/21 11:32 AM, Wang, Kevin(Yang) wrote:

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

We don't use locks for other rings though.

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

This patch is for consistency. It is not fixing any issue/bug.

Nirmoy

thanks.

Best Regards,

Kevin

---

From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> on behalf of Nirmoy Das <nirmoy.das@xxxxxxx>
Sent: Saturday, March 13, 2021 1:08 AM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>
Cc: Das, Nirmoy <Nirmoy.Das@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
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 <nirmoy.das@xxxxxxx>
---
 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
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url="">

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