* [PATCH 1/3] drm/amdgpu: revert "add new bo flag that indicates BOs don't need fallback (v2)" @ 2018-04-10 13:42 Christian König [not found] ` <20180410134240.9515-1-christian.koenig-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2018-04-10 13:42 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW This reverts commit 6f51d28bfe8e1a676de5cd877639245bed3cc818. Makes fallback handling to complicated. This is just a feature for the GEM interface and shouldn't leak into the core BO create function. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +---- include/uapi/drm/amdgpu_drm.h | 2 -- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 68af2f878bc9..e1756b68a17b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -385,8 +385,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, amdgpu_bo_in_cpu_visible_vram(bo)) p->bytes_moved_vis += ctx.bytes_moved; - if (unlikely(r == -ENOMEM) && domain != bo->allowed_domains && - !(bo->flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) { + if (unlikely(r == -ENOMEM) && domain != bo->allowed_domains) { domain = bo->allowed_domains; goto retry; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 9e23d6f6f3f3..04d6830347ec 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -388,8 +388,6 @@ 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 = preferred_domains; - bo->allowed_domains = allowed_domains; bo->flags = flags; @@ -426,8 +424,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size, r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type, &bo->placement, page_align, &ctx, acc_size, NULL, resv, &amdgpu_ttm_bo_destroy); - if (unlikely(r && r != -ERESTARTSYS) && type == ttm_bo_type_device && - !(flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) { + if (unlikely(r && r != -ERESTARTSYS) && type == ttm_bo_type_device) { if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) { flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; goto retry; diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index 80665715e651..0087799962cf 100644 --- a/include/uapi/drm/amdgpu_drm.h +++ b/include/uapi/drm/amdgpu_drm.h @@ -98,8 +98,6 @@ extern "C" { #define AMDGPU_GEM_CREATE_VM_ALWAYS_VALID (1 << 6) /* Flag that BO sharing will be explicitly synchronized */ #define AMDGPU_GEM_CREATE_EXPLICIT_SYNC (1 << 7) -/* Flag that BO doesn't need fallback */ -#define AMDGPU_GEM_CREATE_NO_FALLBACK (1 << 8) struct drm_amdgpu_gem_create_in { /** the requested memory size */ -- 2.14.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20180410134240.9515-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>]
* [PATCH 2/3] drm/amdgpu: revert "Don't change preferred domian when fallback GTT v6" [not found] ` <20180410134240.9515-1-christian.koenig-5C7GfCeVMHo@public.gmane.org> @ 2018-04-10 13:42 ` Christian König 2018-04-10 13:42 ` [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling Christian König 1 sibling, 0 replies; 10+ messages in thread From: Christian König @ 2018-04-10 13:42 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW This reverts commit 7d1ca1325260a9e9329b10a21e3692e6f188936f. Makes fallback handling to complicated. This is just a feature for the GEM interface and shouldn't leak into the core BO create function. The intended change to preserve the preferred domains is implemented in a follow up patch. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 +++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 37 +++++++++++------------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 28c2706e48d7..46b9ea4e6103 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -56,11 +56,23 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size, alignment = PAGE_SIZE; } +retry: r = amdgpu_bo_create(adev, size, alignment, initial_domain, flags, type, resv, &bo); if (r) { - DRM_DEBUG("Failed to allocate GEM object (%ld, %d, %u, %d)\n", - size, initial_domain, alignment, 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); + } 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 04d6830347ec..6d08cde8443c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -356,7 +356,6 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size, struct amdgpu_bo *bo; unsigned long page_align; size_t acc_size; - u32 domains, preferred_domains, allowed_domains; int r; page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT; @@ -370,24 +369,22 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size, acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size, sizeof(struct amdgpu_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); - allowed_domains = preferred_domains; - if (type != ttm_bo_type_kernel && - allowed_domains == AMDGPU_GEM_DOMAIN_VRAM) - allowed_domains |= AMDGPU_GEM_DOMAIN_GTT; - domains = preferred_domains; -retry: bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); if (bo == NULL) return -ENOMEM; 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->flags = flags; @@ -420,20 +417,12 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size, #endif bo->tbo.bdev = &adev->mman.bdev; - amdgpu_ttm_placement_from_domain(bo, domains); + amdgpu_ttm_placement_from_domain(bo, domain); + r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type, &bo->placement, page_align, &ctx, acc_size, NULL, resv, &amdgpu_ttm_bo_destroy); - if (unlikely(r && r != -ERESTARTSYS) && type == ttm_bo_type_device) { - if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) { - flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; - goto retry; - } else if (domains != allowed_domains) { - domains = allowed_domains; - goto retry; - } - } - if (unlikely(r)) + if (unlikely(r != 0)) return r; if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size && -- 2.14.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <20180410134240.9515-1-christian.koenig-5C7GfCeVMHo@public.gmane.org> 2018-04-10 13:42 ` [PATCH 2/3] drm/amdgpu: revert "Don't change preferred domian when fallback GTT v6" Christian König @ 2018-04-10 13:42 ` Christian König [not found] ` <20180410134240.9515-3-christian.koenig-5C7GfCeVMHo@public.gmane.org> 1 sibling, 1 reply; 10+ messages in thread From: Christian König @ 2018-04-10 13:42 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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; + *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 -- 2.14.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20180410134240.9515-3-christian.koenig-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <20180410134240.9515-3-christian.koenig-5C7GfCeVMHo@public.gmane.org> @ 2018-04-11 2:38 ` zhoucm1 [not found] ` <06295e11-12f1-6ca9-133e-f906cdf04ed7-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: zhoucm1 @ 2018-04-11 2:38 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <06295e11-12f1-6ca9-133e-f906cdf04ed7-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <06295e11-12f1-6ca9-133e-f906cdf04ed7-5C7GfCeVMHo@public.gmane.org> @ 2018-04-11 9:22 ` Christian König [not found] ` <29b09ef5-a88a-daff-fc13-c875f975e26e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2018-04-11 9:22 UTC (permalink / raw) To: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 11.04.2018 um 04:38 schrieb zhoucm1: > > > 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? 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.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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <29b09ef5-a88a-daff-fc13-c875f975e26e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <29b09ef5-a88a-daff-fc13-c875f975e26e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2018-04-16 8:57 ` Christian König [not found] ` <47328a0b-e56b-24fb-0838-271a5127a3bc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2018-04-16 8:57 UTC (permalink / raw) To: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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: >>> 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? > > 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? I agree that neither solution is perfect, but updating the placement after we created the BO still sounds like the least painful to me. Christian. > > 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.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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <47328a0b-e56b-24fb-0838-271a5127a3bc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <47328a0b-e56b-24fb-0838-271a5127a3bc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2018-04-16 9:04 ` zhoucm1 [not found] ` <845e35b3-3b2c-0ed0-ba27-9eb4e603df21-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: zhoucm1 @ 2018-04-16 9:04 UTC (permalink / raw) To: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW 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: >>>> 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? >> >> 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 Zhou > > I agree that neither solution is perfect, but updating the placement > after we created the BO still sounds like the least painful to me. > > Christian. > >> >> 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.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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <845e35b3-3b2c-0ed0-ba27-9eb4e603df21-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <845e35b3-3b2c-0ed0-ba27-9eb4e603df21-5C7GfCeVMHo@public.gmane.org> @ 2018-04-16 9:21 ` zhoucm1 [not found] ` <0df2d99a-a997-3842-e063-18543cd49aa2-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: zhoucm1 @ 2018-04-16 9:21 UTC (permalink / raw) To: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2018年04月16日 17:04, zhoucm1 wrote: > > > 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: >>>>> 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? >>> >>> 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 Zhou >> >> I agree that neither solution is perfect, but updating the placement >> after we created the BO still sounds like the least painful to me. And I'm going to have patch for amdgpu_bo_create paramters, collect many paramters to one param struct like done in vm. Regards, David Zhou >> >> Christian. >> >>> >>> 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.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 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <0df2d99a-a997-3842-e063-18543cd49aa2-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <0df2d99a-a997-3842-e063-18543cd49aa2-5C7GfCeVMHo@public.gmane.org> @ 2018-04-16 9:36 ` Christian König [not found] ` <f961f48e-7b37-d988-144c-5f6cfaaa9ec0-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2018-04-16 9:36 UTC (permalink / raw) To: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 16.04.2018 um 11:21 schrieb zhoucm1: > > > On 2018年04月16日 17:04, zhoucm1 wrote: >> >> >> 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: >>>>>> 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? >>>> >>>> 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 Zhou >>> >>> I agree that neither solution is perfect, but updating the placement >>> after we created the BO still sounds like the least painful to me. > And I'm going to have patch for amdgpu_bo_create paramters, collect > many paramters to one param struct like done in vm. 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? Thanks, Christian. > > Regards, > David Zhou >>> >>> Christian. >>> >>>> >>>> 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.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 > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <f961f48e-7b37-d988-144c-5f6cfaaa9ec0-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling [not found] ` <f961f48e-7b37-d988-144c-5f6cfaaa9ec0-5C7GfCeVMHo@public.gmane.org> @ 2018-04-16 10:38 ` zhoucm1 0 siblings, 0 replies; 10+ messages in thread From: zhoucm1 @ 2018-04-16 10:38 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2018年04月16日 17:36, Christian König wrote: > Am 16.04.2018 um 11:21 schrieb zhoucm1: >> >> >> On 2018年04月16日 17:04, zhoucm1 wrote: >>> >>> >>> 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: >>>>>>> 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? >>>>> >>>>> 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 Zhou >>>> >>>> I agree that neither solution is perfect, but updating the >>>> placement after we created the BO still sounds like the least >>>> painful to me. >> And I'm going to have patch for amdgpu_bo_create paramters, collect >> many paramters to one param struct like done in vm. > > 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? 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. Regards, David Zhou > > Thanks, > Christian. > >> >> Regards, >> David Zhou >>>> >>>> Christian. >>>> >>>>> >>>>> 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.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 >> > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-16 10:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-10 13:42 [PATCH 1/3] drm/amdgpu: revert "add new bo flag that indicates BOs don't need fallback (v2)" Christian König [not found] ` <20180410134240.9515-1-christian.koenig-5C7GfCeVMHo@public.gmane.org> 2018-04-10 13:42 ` [PATCH 2/3] drm/amdgpu: revert "Don't change preferred domian when fallback GTT v6" Christian König 2018-04-10 13:42 ` [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling Christian König [not found] ` <20180410134240.9515-3-christian.koenig-5C7GfCeVMHo@public.gmane.org> 2018-04-11 2:38 ` zhoucm1 [not found] ` <06295e11-12f1-6ca9-133e-f906cdf04ed7-5C7GfCeVMHo@public.gmane.org> 2018-04-11 9:22 ` Christian König [not found] ` <29b09ef5-a88a-daff-fc13-c875f975e26e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2018-04-16 8:57 ` Christian König [not found] ` <47328a0b-e56b-24fb-0838-271a5127a3bc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2018-04-16 9:04 ` zhoucm1 [not found] ` <845e35b3-3b2c-0ed0-ba27-9eb4e603df21-5C7GfCeVMHo@public.gmane.org> 2018-04-16 9:21 ` zhoucm1 [not found] ` <0df2d99a-a997-3842-e063-18543cd49aa2-5C7GfCeVMHo@public.gmane.org> 2018-04-16 9:36 ` Christian König [not found] ` <f961f48e-7b37-d988-144c-5f6cfaaa9ec0-5C7GfCeVMHo@public.gmane.org> 2018-04-16 10:38 ` zhoucm1
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.