Am 01.02.2018 um 06:44 schrieb Chunming Zhou:
>
>
> On 2018å¹´01æ??31æ?¥ 23:47, Christian König wrote:
>> The variable is protected by the VMID mutex anyway.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 10 +++++-----
>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h |Â 2 +-
>> Â 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> index fbe958f7cb5b..8374fe870e8c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
>> @@ -267,7 +267,7 @@ static int
>> amdgpu_vmid_grab_reserved_locked(struct amdgpu_vm *vm,
>>       flushed = id->flushed_updates;
>> Â Â Â Â Â if ((amdgpu_vmid_had_gpu_reset(adev, id)) ||
>> -Â Â Â Â Â Â Â (atomic64_read(&id->owner) != vm->entity.fence_context) ||
>> +Â Â Â Â Â Â Â (id->owner != vm->entity.fence_context) ||
>> Â Â Â Â Â Â Â Â Â (job->vm_pd_addr != id->pd_gpu_addr) ||
>> Â Â Â Â Â Â Â Â Â (updates && (!flushed || updates->context !=
>> flushed->context ||
>> Â Â Â Â Â Â Â Â Â Â Â Â Â dma_fence_is_later(updates, flushed))) ||
>> @@ -296,7 +296,7 @@ static int
>> amdgpu_vmid_grab_reserved_locked(struct amdgpu_vm *vm,
>> Â Â Â Â Â Â Â Â Â id->flushed_updates = dma_fence_get(updates);
>> Â Â Â Â Â }
>> Â Â Â Â Â id->pd_gpu_addr = job->vm_pd_addr;
>> -Â Â Â atomic64_set(&id->owner, vm->entity.fence_context);
>> +Â Â Â id->owner = vm->entity.fence_context;
>> Â Â Â Â Â job->vm_needs_flush = needs_flush;
>> Â Â Â Â Â if (needs_flush) {
>> Â Â Â Â Â Â Â Â Â dma_fence_put(id->last_flush);
>> @@ -353,7 +353,7 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct
>> amdgpu_ring *ring,
>> Â Â Â Â Â Â Â Â Â if (amdgpu_vmid_had_gpu_reset(adev, id))
>> Â Â Â Â Â Â Â Â Â Â Â Â Â continue;
>> Â -Â Â Â Â Â Â Â if (atomic64_read(&id->owner) != vm->entity.fence_context)
>> +Â Â Â Â Â Â Â if (id->owner != vm->entity.fence_context)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â continue;
>> Â Â Â Â Â Â Â Â Â Â if (job->vm_pd_addr != id->pd_gpu_addr)
>> @@ -402,7 +402,7 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct
>> amdgpu_ring *ring,
>> Â Â Â Â Â id->pd_gpu_addr = job->vm_pd_addr;
>> Â Â Â Â Â dma_fence_put(id->flushed_updates);
>> Â Â Â Â Â id->flushed_updates = dma_fence_get(updates);
>> -Â Â Â atomic64_set(&id->owner, vm->entity.fence_context);
>> +Â Â Â id->owner = vm->entity.fence_context;
>> Â Â needs_flush:
>> Â Â Â Â Â job->vm_needs_flush = true;
>> @@ -482,7 +482,7 @@ void amdgpu_vmid_reset(struct amdgpu_device
>> *adev, unsigned vmhub,
>> Â Â Â Â Â struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
>> Â Â Â Â Â struct amdgpu_vmid *id = &id_mgr->ids[vmid];
>> Â -Â Â Â atomic64_set(&id->owner, 0);
>> +Â Â Â id->owner = 0;
> here is not protect by id_mgr mutex, you can make another patch to add
> mutex for this function.
That function is only called in case of suspend/resume and GPU reset,
but adding the extra mutex here probably won't hurt and is indeed cleaner.
Christian.
>
> Regards,
> David Zhou
>> Â Â Â Â Â id->gds_base = 0;
>> Â Â Â Â Â id->gds_size = 0;
>> Â Â Â Â Â id->gws_base = 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>> index 38f37c16fc5e..20d4eca6cd6a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
>> @@ -43,7 +43,7 @@ struct amdgpu_vmid {
>>      struct list_head   list;
>>      struct amdgpu_sync   active;
>>      struct dma_fence   *last_flush;
>> -   atomic64_t       owner;
>> +   uint64_t       owner;
>>       uint64_t       pd_gpu_addr;
>> Â Â Â Â Â /* last flushed PD/PT update */
>
