Re: [PATCH v2 2/4] drm/amdgpu: Allow explicit sync for VM ops.

["Christian König" <christian.koenig@amd.com>]

Am 21.08.23 um 08:20 schrieb Tatsuyuki Ishi:
> From: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>
> This should be okay because moves themselves use KERNEL usage and
> hence still sync with BOOKKEEP usage. Then any later submits still
> wait on any pending VM operations.
>
> (i.e. we only made VM ops not wait on BOOKKEEP submits, not the other
>   way around)

Well that approach won't work like this.

Basically the whole approach with the implicit vs. explicit handling 
sounds like a really bad idea to me.

What you need to do instead is to give amdgpu_vm_sdma_prepare() and 
amdgpu_vm_cpu_prepare() the information if this is an update triggered 
by the kernel (because the BO was evicted) or triggered by userspace.

Take a look where the sync_mode parameter is determined instead.

Regards,
Christian.

>
> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> Signed-off-by: Tatsuyuki Ishi <ishitatsuyuki@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c  | 3 ++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> index f10332e1c6c0..e898a549f86d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> @@ -51,7 +51,8 @@ static int amdgpu_vm_cpu_prepare(struct amdgpu_vm_update_params *p,
>   	if (!resv)
>   		return 0;
>   
> -	return amdgpu_bo_sync_wait_resv(p->adev, resv, sync_mode, sync_mode, p->vm, true);
> +	return amdgpu_bo_sync_wait_resv(p->adev, resv, sync_mode,
> +					AMDGPU_SYNC_EXPLICIT, p->vm, true);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index e259a51e7c56..8cb427710d66 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -98,7 +98,8 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
>   		return 0;
>   
>   	amdgpu_sync_create(&sync);
> -	r = amdgpu_sync_resv(p->adev, &sync, resv, sync_mode, sync_mode, p->vm);
> +	r = amdgpu_sync_resv(p->adev, &sync, resv, sync_mode,
> +				AMDGPU_SYNC_EXPLICIT, p->vm);
>   	if (!r)
>   		r = amdgpu_sync_push_to_job(&sync, p->job);
>   	amdgpu_sync_free(&sync);