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

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

[Nirmoy Das]

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@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 1915b9b95106..8fe370e5358d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -467,13 +467,17 @@ int amdgpu_gfx_disable_kcq(struct amdgpu_device *adev)
 	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);
+	spin_unlock(&adev->gfx.kiq.ring_lock);
 
 	return amdgpu_ring_test_helper(kiq_ring);
 }
@@ -518,18 +522,20 @@ 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;
 	}
 
 	kiq->pmf->kiq_set_resources(kiq_ring, queue_mask);
 	for (i = 0; i < adev->gfx.num_compute_rings; i++)
 		kiq->pmf->kiq_map_queues(kiq_ring, &adev->gfx.compute_ring[i]);
+	spin_unlock(&adev->gfx.kiq.ring_lock);
 
 	r = amdgpu_ring_test_helper(kiq_ring);
 	if (r)
-- 
2.30.1

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

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

[Christian König]

Am 12.03.21 um 10:49 schrieb Nirmoy Das:
> 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.

Ah, now I knew which functions you mean. This is not strictly necessary 
because that stuff is only called during bootup and not later on.

>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 1915b9b95106..8fe370e5358d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -467,13 +467,17 @@ int amdgpu_gfx_disable_kcq(struct amdgpu_device *adev)
>   	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);
> +	spin_unlock(&adev->gfx.kiq.ring_lock);
>   
>   	return amdgpu_ring_test_helper(kiq_ring);

The ring test accesses the ring buffer as well.

Regards,
Christian.

>   }
> @@ -518,18 +522,20 @@ 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;
>   	}
>   
>   	kiq->pmf->kiq_set_resources(kiq_ring, queue_mask);
>   	for (i = 0; i < adev->gfx.num_compute_rings; i++)
>   		kiq->pmf->kiq_map_queues(kiq_ring, &adev->gfx.compute_ring[i]);
> +	spin_unlock(&adev->gfx.kiq.ring_lock);
>   
>   	r = amdgpu_ring_test_helper(kiq_ring);
>   	if (r)

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

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

[Nirmoy]

On 3/12/21 10:52 AM, Christian König wrote:
> Am 12.03.21 um 10:49 schrieb Nirmoy Das:
>> 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.
>
> Ah, now I knew which functions you mean. This is not strictly 
> necessary because that stuff is only called during bootup and not 
> later on.


OK, not so serious issue as I was thinking then.


>
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index 1915b9b95106..8fe370e5358d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -467,13 +467,17 @@ int amdgpu_gfx_disable_kcq(struct amdgpu_device 
>> *adev)
>>       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);
>> +    spin_unlock(&adev->gfx.kiq.ring_lock);
>>         return amdgpu_ring_test_helper(kiq_ring);
>
> The ring test accesses the ring buffer as well.


Shall I send a v2 ?


Thanks,

Nirmoy


>
>
> Regards,
> Christian.
>
>>   }
>> @@ -518,18 +522,20 @@ 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;
>>       }
>>         kiq->pmf->kiq_set_resources(kiq_ring, queue_mask);
>>       for (i = 0; i < adev->gfx.num_compute_rings; i++)
>>           kiq->pmf->kiq_map_queues(kiq_ring, 
>> &adev->gfx.compute_ring[i]);
>> +    spin_unlock(&adev->gfx.kiq.ring_lock);
>>         r = amdgpu_ring_test_helper(kiq_ring);
>>       if (r)
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Christian König]

Am 12.03.21 um 11:24 schrieb Nirmoy:
>
> On 3/12/21 10:52 AM, Christian König wrote:
>> Am 12.03.21 um 10:49 schrieb Nirmoy Das:
>>> 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.
>>
>> Ah, now I knew which functions you mean. This is not strictly 
>> necessary because that stuff is only called during bootup and not 
>> later on.
>
>
> OK, not so serious issue as I was thinking then.
>
>
>>
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 10 ++++++++--
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> index 1915b9b95106..8fe370e5358d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> @@ -467,13 +467,17 @@ int amdgpu_gfx_disable_kcq(struct 
>>> amdgpu_device *adev)
>>>       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);
>>> +    spin_unlock(&adev->gfx.kiq.ring_lock);
>>>         return amdgpu_ring_test_helper(kiq_ring);
>>
>> The ring test accesses the ring buffer as well.
>
>
> Shall I send a v2 ?

I'm not 100% sure. Please test this with lockdep enabled, if that 
doesn't raise anything we can probably commit it just to be on the clean 
side.

Regards,
Christian.

>
>
> Thanks,
>
> Nirmoy
>
>
>>
>>
>> Regards,
>> Christian.
>>
>>>   }
>>> @@ -518,18 +522,20 @@ 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;
>>>       }
>>>         kiq->pmf->kiq_set_resources(kiq_ring, queue_mask);
>>>       for (i = 0; i < adev->gfx.num_compute_rings; i++)
>>>           kiq->pmf->kiq_map_queues(kiq_ring, 
>>> &adev->gfx.compute_ring[i]);
>>> +    spin_unlock(&adev->gfx.kiq.ring_lock);
>>>         r = amdgpu_ring_test_helper(kiq_ring);
>>>       if (r)
>>

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

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

[Nirmoy]

[-- Attachment #1.1: Type: text/plain, Size: 4637 bytes --]


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@lists.freedesktop.org> on behalf of 
> Nirmoy Das <nirmoy.das@amd.com>
> *Sent:* Saturday, March 13, 2021 1:08 AM
> *To:* Koenig, Christian <Christian.Koenig@amd.com>
> *Cc:* Das, Nirmoy <Nirmoy.Das@amd.com>; amd-gfx@lists.freedesktop.org 
> <amd-gfx@lists.freedesktop.org>
> *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@amd.com>
> ---
>  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@lists.freedesktop.org
> 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 
> <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>

[-- Attachment #1.2: Type: text/html, Size: 15068 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

[Wang, Kevin(Yang)]

[-- Attachment #1.1: Type: text/plain, Size: 3969 bytes --]

[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 <amd-gfx-bounces@lists.freedesktop.org> on behalf of Nirmoy Das <nirmoy.das@amd.com>
Sent: Saturday, March 13, 2021 1:08 AM
To: Koenig, Christian <Christian.Koenig@amd.com>
Cc: Das, Nirmoy <Nirmoy.Das@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
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@amd.com>
---
 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@lists.freedesktop.org
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

[-- Attachment #1.2: Type: text/html, Size: 11191 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

[Christian König]

Am 12.03.21 um 18:08 schrieb Nirmoy Das:
> 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@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   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");
>   

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

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

[Nirmoy Das]

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@amd.com>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx