drm/amdgpu: Don't change preferred domian when fallback GTT v4

Submitted by Zhou, David(ChunMing) on March 20, 2018, 7:55 a.m.

Details

Message ID 20180320075515.29986-1-david1.zhou@amd.com
State New
Headers show
Series "drm/amdgpu: Don't change preferred domian when fallback GTT v4" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Zhou, David(ChunMing) March 20, 2018, 7:55 a.m.
v2: add sanity checking
v3: make code open
v4: also handle visible to invisible fallback

Change-Id: I2cf672ad36b8b4cc1a6b2e704f786bf6a155d9ce
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 16 ++--------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 ++++++++++++++++---
 2 files changed, 18 insertions(+), 17 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 6e6570ff9f8b..8328684aee06 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -76,23 +76,11 @@  int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
 		}
 	}
 
-retry:
 	r = amdgpu_bo_create(adev, size, alignment, kernel, initial_domain,
 			     flags, NULL, resv, &bo);
 	if (r) {
-		if (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 (%ld, %d, %u, %d)\n",
-				  size, initial_domain, alignment, r);
-		}
+		DRM_DEBUG("Failed to allocate GEM object (%ld, %d, %u, %d)\n",
+			  size, initial_domain, alignment, r);
 		return r;
 	}
 	*obj = &bo->gem_base;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index b3310219e0ac..84c5e9db1b39 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -440,12 +440,25 @@  static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 #endif
 
 	bo->tbo.bdev = &adev->mman.bdev;
-	amdgpu_ttm_placement_from_domain(bo, domain);
-
+retry:
+	amdgpu_ttm_placement_from_domain(bo, bo->preferred_domains);
 	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type,
 				 &bo->placement, page_align, &ctx, acc_size,
 				 sg, resv, &amdgpu_ttm_bo_destroy);
-	if (unlikely(r != 0))
+
+	if (unlikely(r && r != -ERESTARTSYS)) {
+	    if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
+		    bo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+		    goto retry;
+	    } else if (bo->allowed_domains != bo->preferred_domains) {
+		    amdgpu_ttm_placement_from_domain(bo, bo->allowed_domains);
+		    r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size,
+					     type, &bo->placement, page_align,
+					     &ctx, acc_size, sg, resv,
+					     &amdgpu_ttm_bo_destroy);
+	    }
+	}
+	if (unlikely(r))
 		return r;
 
 	if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&

Comments

Am 20.03.2018 um 08:55 schrieb Chunming Zhou:
> v2: add sanity checking
> v3: make code open
> v4: also handle visible to invisible fallback
>
> Change-Id: I2cf672ad36b8b4cc1a6b2e704f786bf6a155d9ce
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 16 ++--------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 ++++++++++++++++---
>   2 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6e6570ff9f8b..8328684aee06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -76,23 +76,11 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   		}
>   	}
>   
> -retry:
>   	r = amdgpu_bo_create(adev, size, alignment, kernel, initial_domain,
>   			     flags, NULL, resv, &bo);
>   	if (r) {
> -		if (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 (%ld, %d, %u, %d)\n",
> -				  size, initial_domain, alignment, r);
> -		}
> +		DRM_DEBUG("Failed to allocate GEM object (%ld, %d, %u, %d)\n",
> +			  size, initial_domain, alignment, r);
>   		return r;
>   	}
>   	*obj = &bo->gem_base;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index b3310219e0ac..84c5e9db1b39 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -440,12 +440,25 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   #endif
>   
>   	bo->tbo.bdev = &adev->mman.bdev;
> -	amdgpu_ttm_placement_from_domain(bo, domain);
> -
> +retry:
> +	amdgpu_ttm_placement_from_domain(bo, bo->preferred_domains);
>   	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type,
>   				 &bo->placement, page_align, &ctx, acc_size,
>   				 sg, resv, &amdgpu_ttm_bo_destroy);
> -	if (unlikely(r != 0))
> +
> +	if (unlikely(r && r != -ERESTARTSYS)) {
> +	    if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> +		    bo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +		    goto retry;
> +	    } else if (bo->allowed_domains != bo->preferred_domains) {
> +		    amdgpu_ttm_placement_from_domain(bo, bo->allowed_domains);
> +		    r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size,
> +					     type, &bo->placement, page_align,
> +					     &ctx, acc_size, sg, resv,
> +					     &amdgpu_ttm_bo_destroy);
> +	    }
> +	}
> +	if (unlikely(r))

Mhm, again this ugly retry label. But since we now handled two cases 
open coding this becomes to lengthly as well.

Let's go back to your original approach. How about the following code:

domains = bo->preferred_domains;
retry:
     amdgpu_ttm_placement_from_domain(bo, domains);
     r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type,
         &bo->placement, page_align, &ctx, acc_size,
         sg, resv, &amdgpu_ttm_bo_destroy);

     if (unlikely(r && r != -ERESTARTSYS)) {
         if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
             bo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
             goto retry;
         } else if (domains != bo->preferred_domains) {
             domains = bo->preferred_domains;
             goto retry;
         }
     }
     if (unlikely(r))
...

That shouldn't loop even if it fails with preferred_domains and handle 
the case gracefully that we first try to clear the flag and then the 
move the BO to GTT.

Regards,
Christian.