Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org>]

On 2018年04月10日 21:42, Christian König wrote:
> When GEM needs to fallback to GTT for VRAM BOs we still want the
> preferred domain to be untouched so that the BO has a cance to move back
> to VRAM in the future.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 14 +++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++-----------
>   2 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 46b9ea4e6103..9dc0a190413c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   			     struct reservation_object *resv,
>   			     struct drm_gem_object **obj)
>   {
> +	uint32_t domain = initial_domain;
>   	struct amdgpu_bo *bo;
>   	int r;
>   
> @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   	}
>   
>   retry:
> -	r = amdgpu_bo_create(adev, size, alignment, initial_domain,
> +	r = amdgpu_bo_create(adev, size, alignment, domain,
>   			     flags, type, resv, &bo);
>   	if (r) {
>   		if (r != -ERESTARTSYS) {
> @@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   				goto retry;
>   			}
>   
> -			if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
> -				initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
> +			if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
> +				domain |= AMDGPU_GEM_DOMAIN_GTT;
>   				goto retry;
>   			}
>   			DRM_DEBUG("Failed to allocate GEM object (%ld, %d, %u, %d)\n",
> @@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   		}
>   		return r;
>   	}
> +
> +	bo->preferred_domains = initial_domain;
> +	bo->allowed_domains = initial_domain;
> +	if (type != ttm_bo_type_kernel &&
> +	    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
> +		bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
It's an ugly change back after bo creation! Do you think it's better 
than previous?

Alternative way, you can add one parameter to amdgpu_bo_create, directly 
pass preferred_domains and allowed_domains to amdgpu_bo_create.

Regards,
David Zhou
> +
>   	*obj = &bo->gem_base;
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6d08cde8443c..854d18d5cff4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
>   	drm_gem_private_object_init(adev->ddev, &bo->gem_base, size);
>   	INIT_LIST_HEAD(&bo->shadow_list);
>   	INIT_LIST_HEAD(&bo->va);
> -	bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
> -					 AMDGPU_GEM_DOMAIN_GTT |
> -					 AMDGPU_GEM_DOMAIN_CPU |
> -					 AMDGPU_GEM_DOMAIN_GDS |
> -					 AMDGPU_GEM_DOMAIN_GWS |
> -					 AMDGPU_GEM_DOMAIN_OA);
> -	bo->allowed_domains = bo->preferred_domains;
> -	if (type != ttm_bo_type_kernel &&
> -	    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
> -		bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
> -
> +	bo->preferred_domains = domain;
> +	bo->allowed_domains = domain;
>   	bo->flags = flags;
>   
>   #ifdef CONFIG_X86_32

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx