Re: [PATCH 4/6] drm/amdgpu: use drm_exec for GEM and CSA handling

[Tatsuyuki Ishi <ishitatsuyuki@xxxxxxxxx>]

On 6/28/23 19:44, Christian König wrote:

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 74055cba3dc9..6811fc866494 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -728,36 +723,37 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>   		return -EINVAL;
>   	}
>   
> -	INIT_LIST_HEAD(&list);
> -	INIT_LIST_HEAD(&duplicates);
>   	if ((args->operation != AMDGPU_VA_OP_CLEAR) &&
>   	    !(args->flags & AMDGPU_VM_PAGE_PRT)) {
>   		gobj = drm_gem_object_lookup(filp, args->handle);
>   		if (gobj == NULL)
>   			return -ENOENT;
>   		abo = gem_to_amdgpu_bo(gobj);
> -		tv.bo = &abo->tbo;
> -		if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)
> -			tv.num_shared = 1;
> -		else
> -			tv.num_shared = 0;
> -		list_add(&tv.head, &list);
>   	} else {
>   		gobj = NULL;
>   		abo = NULL;
>   	}
>   
> -	amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd);
> +	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);

Sorry, I missed this last time, but this needs to allow duplicates as well or mapping
always_valid BOs doesn't work.

> +	drm_exec_until_all_locked(&exec) {
> +		if (gobj) {
> +			r = drm_exec_lock_obj(&exec, gobj);
> +			drm_exec_retry_on_contention(&exec);
> +			if (unlikely(r))
> +				goto error;
> +		}
>   
> -	r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates);
> -	if (r)
> -		goto error_unref;
> +		r = amdgpu_vm_lock_pd(&fpriv->vm, &exec, 2);
> +		drm_exec_retry_on_contention(&exec);
> +		if (unlikely(r))
> +			goto error;
> +	}
>   
>   	if (abo) {
>   		bo_va = amdgpu_vm_bo_find(&fpriv->vm, abo);
>   		if (!bo_va) {
>   			r = -ENOENT;
> -			goto error_backoff;
> +			goto error;
>   		}
>   	} else if (args->operation != AMDGPU_VA_OP_CLEAR) {
>   		bo_va = fpriv->prt_va;
> @@ -794,10 +790,8 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>   		amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va,
>   					args->operation);
>   
> -error_backoff:
> -	ttm_eu_backoff_reservation(&ticket, &list);
> -
> -error_unref:
> +error:
> +	drm_exec_fini(&exec);
>   	drm_gem_object_put(gobj);
>   	return r;
>   }