Re: [PATCH] drm/amdgpu: revert "generally allow over-commit during BO allocation"

[Felix Kuehling <felix.kuehling@xxxxxxx>]

On 2022-12-12 11:33, Christian König wrote:
> This reverts commit 4ec11490081bcbc4b90d20622cd211c6eba8d6fc.
>
> This causes problem for KFD because when we overcommit we accidentially
> bind the BO to GTT for moving it into VRAM. We also need to make sure
> that this is done only as fallback after trying to evict first.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>

Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 16 +++++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +++++-
>   2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 62e98f1ad770..a0780a4e3e61 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -113,7 +113,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   	bp.resv = resv;
>   	bp.preferred_domain = initial_domain;
>   	bp.flags = flags;
> -	bp.domain = initial_domain | AMDGPU_GEM_DOMAIN_CPU;
> +	bp.domain = initial_domain;
>   	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>   
>   	r = amdgpu_bo_create_user(adev, &bp, &ubo);
> @@ -332,10 +332,20 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   	}
>   
>   	initial_domain = (u32)(0xffffffff & args->in.domains);
> +retry:
>   	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
> -				     initial_domain, flags, ttm_bo_type_device,
> -				     resv, &gobj);
> +				     initial_domain,
> +				     flags, ttm_bo_type_device, resv, &gobj);
>   	if (r && r != -ERESTARTSYS) {
> +		if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> +			flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +			goto retry;
> +		}
> +
> +		if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
> +			initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
> +			goto retry;
> +		}
>   		DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu, %d)\n",
>   				size, initial_domain, args->in.alignment, r);
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index fd3ab4b5e5bb..871f7c136de5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -582,7 +582,11 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   		bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
>   
>   	bo->tbo.bdev = &adev->mman.bdev;
> -	amdgpu_bo_placement_from_domain(bo, bp->domain);
> +	if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
> +			  AMDGPU_GEM_DOMAIN_GDS))
> +		amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
> +	else
> +		amdgpu_bo_placement_from_domain(bo, bp->domain);
>   	if (bp->type == ttm_bo_type_kernel)
>   		bo->tbo.priority = 1;
>