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 at 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.
