[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 at 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)
>