On 2018年04月16日 17:36, Christian König wrote:
you can push your reverting patch#1 and #2 first. we can re-do this patch#3 again after I pass my patches of collecting parameters for amdgpu_bo_create.Am 16.04.2018 um 11:21 schrieb zhoucm1:On 2018年04月16日 17:04, zhoucm1 wrote:And I'm going to have patch for amdgpu_bo_create paramters, collect many paramters to one param struct like done in vm.On 2018年04月16日 16:57, Christian König wrote:Am 11.04.2018 um 11:22 schrieb Christian König: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 thepreferred domain to be untouched so that the BO has a cance to move backto VRAM in the future. Signed-off-by: Christian König <[email protected]> --- 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.Ping? Any more opinions on how to proceed?No, I keep my opinion on this. Michel already gave your RB I remember. Regards, David ZhouI agree that neither solution is perfect, but updating the placement after we created the BO still sounds like the least painful to me.Oh, good idea! That would be a good solution as well I think.Then we can provide amdgpu_bo_do_create() with both the preferred and the allowed domain.Do you want to keep working on this or should I?
Regards, David Zhou
Thanks, Christian.Regards, David ZhouChristian.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 [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
