Am 11.04.2018 um 04:38 schrieb zhoucm1:
On 2018年04月10日 21:42, Christian König wrote:It's an ugly change back after bo creation! Do you think it's better than previous?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.koe...@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.cindex 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;
Well it's better than moving the fallback handling into the core functions and then adding a flag to disable it again because we don't want it in the core function.
Alternative way, you can add one parameter to amdgpu_bo_create, directly pass preferred_domains and allowed_domains to amdgpu_bo_create.
Then we would need three parameters, one where to create the BO now and two for preferred/allowed domains.
I think that in this case overwriting the placement from the initial value looks cleaner to me.
Regards, Christian.
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.cindex 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
