On 11/6/25 16:55, Tvrtko Ursulin wrote:
>
> On 06/11/2025 13:06, Christian König wrote:
>> When we run out of VMIDs we need to wait for some to become available.
>> Previously we were using a dma_fence_array for that, but this means that
>> we have to allocate memory.
>>
>> Instead just wait for the first not signaled fence from the least recently
>> used VMID to signal. That is not as efficient since we end up in this
>> function multiple times again, but allocating memory can easily fail or
>> deadlock if we have to wait for memory to become available.
>>
>> Signed-off-by: Christian König <[email protected]>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 51 ++++++-------------------
>> 1 file changed, 12 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> index 3ef5bc95642c..5f94a66511af 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> @@ -201,58 +201,31 @@ static int amdgpu_vmid_grab_idle(struct amdgpu_ring
>> *ring,
>> struct amdgpu_device *adev = ring->adev;
>> unsigned vmhub = ring->vm_hub;
>> struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
>> - struct dma_fence **fences;
>> - unsigned i;
>> + /* If anybody is waiting for a VMID let everybody wait for fairness */
>> if (!dma_fence_is_signaled(ring->vmid_wait)) {
>> *fence = dma_fence_get(ring->vmid_wait);
>> return 0;
>> }
>> - fences = kmalloc_array(id_mgr->num_ids, sizeof(void *), GFP_NOWAIT);
>> - if (!fences)
>> - return -ENOMEM;
>> -
>> /* Check if we have an idle VMID */
>> - i = 0;
>> - list_for_each_entry((*idle), &id_mgr->ids_lru, list) {
>> + list_for_each_entry_reverse((*idle), &id_mgr->ids_lru, list) {
>> /* Don't use per engine and per process VMID at the same time */
>> struct amdgpu_ring *r = adev->vm_manager.concurrent_flush ?
>> NULL : ring;
>> - fences[i] = amdgpu_sync_peek_fence(&(*idle)->active, r);
>> - if (!fences[i])
>> - break;
>> - ++i;
>> - }
>> -
>> - /* If we can't find a idle VMID to use, wait till one becomes available
>> */
>> - if (&(*idle)->list == &id_mgr->ids_lru) {
>> - u64 fence_context = adev->vm_manager.fence_context + ring->idx;
>> - unsigned seqno = ++adev->vm_manager.seqno[ring->idx];
>
> Maybe vm_manager.fence_context && seqno are now unused and can be removed?
Good point!
Thanks,
Christian.
>
> Regards,
>
> Tvrtko
>
>> - struct dma_fence_array *array;
>> - unsigned j;
>> -
>> - *idle = NULL;
>> - for (j = 0; j < i; ++j)
>> - dma_fence_get(fences[j]);
>> -
>> - array = dma_fence_array_create(i, fences, fence_context,
>> - seqno, true);
>> - if (!array) {
>> - for (j = 0; j < i; ++j)
>> - dma_fence_put(fences[j]);
>> - kfree(fences);
>> - return -ENOMEM;
>> - }
>> -
>> - *fence = dma_fence_get(&array->base);
>> - dma_fence_put(ring->vmid_wait);
>> - ring->vmid_wait = &array->base;
>> - return 0;
>> + *fence = amdgpu_sync_peek_fence(&(*idle)->active, r);
>> + if (!(*fence))
>> + return 0;
>> }
>> - kfree(fences);
>> + /*
>> + * If we can't find a idle VMID to use, wait on a fence from the least
>> + * recently used in the hope that it will be available soon.
>> + */
>> + *idle = NULL;
>> + dma_fence_put(ring->vmid_wait);
>> + ring->vmid_wait = dma_fence_get(*fence);
>> return 0;
>> }
>>
>
