[PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
In the conversion AMDGPU_PL_PREEMPT either does not have to be handled
because amdgpu_mem_type_to_domain() cannot return that value anyway.
v2:
* Remove AMDGPU_PL_PREEMPT handling.
v3:
* Rebase.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König # v1
Reviewed-by: Felix Kuehling # v2
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 29 +
2 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..0b3b10d21952 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
-AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8d8c39be6129..4f9073dd19eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -983,12 +983,11 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1293,7 +1292,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
struct ttm_resource *res = bo->tbo.resource;
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1303,21 +1301,20 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(res->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (res->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
- if (amdgpu_res_cpu_visible(adev, bo->tbo.resource))
+ if (amdgpu_res_cpu_visible(adev, res))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1330,7 +1327,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (res->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1604,20 +1601,18 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo,
struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_res_cpu_visible(adev, bo->tbo.resource))
placement = "VRAM VISIBLE";
else
placement = "VRAM";
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case
Re: [PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
Am 29.04.24 um 18:47 schrieb Tvrtko Ursulin:
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
In the conversion AMDGPU_PL_PREEMPT either does not have to be handled
because amdgpu_mem_type_to_domain() cannot return that value anyway.
v2:
* Remove AMDGPU_PL_PREEMPT handling.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König # v1
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 27 +
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..0b3b10d21952 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
-AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..eb5bd6962560 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,11 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1279,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1288,20 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1314,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1589,17 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo,
struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_bo_in_cpu_visible_vram(bo))
placement = "VRAM VISIBLE";
else
placement = "VRAM";
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
placement = "GTT";
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+
Re: [PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
On 30/04/2024 01:31, Felix Kuehling wrote:
On 2024-04-29 12:47, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
In the conversion AMDGPU_PL_PREEMPT either does not have to be handled
because amdgpu_mem_type_to_domain() cannot return that value anyway.
v2:
* Remove AMDGPU_PL_PREEMPT handling.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König # v1
Reviewed-by: Felix Kuehling
I also ran kfdtest on a multi-GPU system just to make sure this didn't
break our multi-GPU support. BTW, I had to fix up some things when I
Excellent thank you!
Btw important thing to stress is that I hope the r-b means not only
patch is functionaly correct but that you guys actually agree it is an
improvement. Because I am quite new in your code base so please apply
strict criteria on my proposals.
tried to apply your patch to the current amd-staging-drm-next branch.
That branch was just rebased on Linux 6.8, so maybe that's part of the
reason.
I am conditioned to work against drm-tip so maybe that is one reason, or
also possibly because now I see I used drm-tip from more than a week ago
as a base. :( I can rebase and re-send. So amd-staging-drm-next is the
correct branch?
Regards,
Tvrtko
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 27 +
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..0b3b10d21952 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
- AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..eb5bd6962560 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,11 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
*bo, u32 domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1279,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1288,20 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1314,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1589,17 @@ u64 amdgpu_bo_print_info(int id, struct
amdgpu_bo *bo, struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain =
Re: [PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
Hi Christian,
On 29/04/2024 12:03, Christian König wrote:
Am 26.04.24 um 18:43 schrieb Tvrtko Ursulin:
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König
Thanks! I am however a bit unsure now, read below..
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 30 ++---
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..ff83f8d8628c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
- AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT &&
+ bo->tbo.resource->mem_type != AMDGPU_PL_PREEMPT) {
return ERR_PTR(-EBUSY);
... this for instance (one hunk below too).
Because what we are discussing in 3/3,
amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) does not return a
valid domain for AMDGPU_PL_PREEMPT. Which could make mentioning it here
misleading. So I am not really sure what the design is supposed to be.
Is this a weakness in the current code? Maybe best to leave the
discussion for 3/3.
Regards,
Tvrtko
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..fb984669fc3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,12 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
*bo, u32 domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT ||
+ bo->tbo.resource->mem_type == AMDGPU_PL_PREEMPT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1280,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1289,21 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
+ case AMDGPU_PL_PREEMPT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1316,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1591,18 @@ u64 amdgpu_bo_print_info(int id, struct
amdgpu_bo *bo, struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_bo_in_cpu_visible_vram(bo))
placement = "VRAM VISIBLE";
else
placement = "VRAM";
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
Re: [PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
On 2024-04-29 12:47, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
In the conversion AMDGPU_PL_PREEMPT either does not have to be handled
because amdgpu_mem_type_to_domain() cannot return that value anyway.
v2:
* Remove AMDGPU_PL_PREEMPT handling.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König # v1
Reviewed-by: Felix Kuehling
I also ran kfdtest on a multi-GPU system just to make sure this didn't
break our multi-GPU support. BTW, I had to fix up some things when I
tried to apply your patch to the current amd-staging-drm-next branch.
That branch was just rebased on Linux 6.8, so maybe that's part of the
reason.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 27 +
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..0b3b10d21952 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
-AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..eb5bd6962560 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,11 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1279,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1288,20 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1314,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1589,17 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo,
struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_bo_in_cpu_visible_vram(bo))
[PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
In the conversion AMDGPU_PL_PREEMPT either does not have to be handled
because amdgpu_mem_type_to_domain() cannot return that value anyway.
v2:
* Remove AMDGPU_PL_PREEMPT handling.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König # v1
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 27 +
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..0b3b10d21952 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
-AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..eb5bd6962560 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,11 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1279,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1288,20 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1314,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1589,17 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo,
struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_bo_in_cpu_visible_vram(bo))
placement = "VRAM VISIBLE";
else
placement = "VRAM";
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
placement = "GTT";
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case
Re: [PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
Am 26.04.24 um 18:43 schrieb Tvrtko Ursulin:
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 30 ++---
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..ff83f8d8628c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
-AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT &&
+ bo->tbo.resource->mem_type != AMDGPU_PL_PREEMPT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..fb984669fc3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,12 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT ||
+ bo->tbo.resource->mem_type == AMDGPU_PL_PREEMPT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1280,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1289,21 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
+ case AMDGPU_PL_PREEMPT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1316,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1591,18 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo,
struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_bo_in_cpu_visible_vram(bo))
placement = "VRAM VISIBLE";
else
placement = "VRAM";
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
+ case AMDGPU_PL_PREEMPT:
placement = "GTT";
break;
-
[PATCH 2/3] drm/amdgpu: Reduce mem_type to domain double indirection
From: Tvrtko Ursulin
All apart from AMDGPU_GEM_DOMAIN_GTT memory domains map 1:1 to TTM
placements. And the former be either AMDGPU_PL_PREEMPT or TTM_PL_TT,
depending on AMDGPU_GEM_CREATE_PREEMPTIBLE.
Simplify a few places in the code which convert the TTM placement into
a domain by checking against the current placement directly.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 30 ++---
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 055ba2ea4c12..ff83f8d8628c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -165,8 +165,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
if (r)
return ERR_PTR(r);
- } else if (!(amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type) &
-AMDGPU_GEM_DOMAIN_GTT)) {
+ } else if (bo->tbo.resource->mem_type != TTM_PL_TT &&
+ bo->tbo.resource->mem_type != AMDGPU_PL_PREEMPT) {
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..fb984669fc3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -976,12 +976,12 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
ttm_bo_pin(>tbo);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type == TTM_PL_VRAM) {
atomic64_add(amdgpu_bo_size(bo), >vram_pin_size);
atomic64_add(amdgpu_vram_mgr_bo_visible_size(bo),
>visible_pin_size);
- } else if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+ } else if (bo->tbo.resource->mem_type == TTM_PL_TT ||
+ bo->tbo.resource->mem_type == AMDGPU_PL_PREEMPT) {
atomic64_add(amdgpu_bo_size(bo), >gart_pin_size);
}
@@ -1280,7 +1280,6 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
{
uint64_t size = amdgpu_bo_size(bo);
struct drm_gem_object *obj;
- unsigned int domain;
bool shared;
/* Abort if the BO doesn't currently have a backing store */
@@ -1290,21 +1289,21 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
obj = >tbo.base;
shared = drm_gem_object_is_shared_for_memory_stats(obj);
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
stats->vram += size;
if (amdgpu_bo_in_cpu_visible_vram(bo))
stats->visible_vram += size;
if (shared)
stats->vram_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
+ case AMDGPU_PL_PREEMPT:
stats->gtt += size;
if (shared)
stats->gtt_shared += size;
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case TTM_PL_SYSTEM:
default:
stats->cpu += size;
if (shared)
@@ -1317,7 +1316,7 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->requested_visible_vram += size;
- if (domain != AMDGPU_GEM_DOMAIN_VRAM) {
+ if (bo->tbo.resource->mem_type != TTM_PL_VRAM) {
stats->evicted_vram += size;
if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
stats->evicted_visible_vram += size;
@@ -1592,19 +1591,18 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo,
struct seq_file *m)
u64 size;
if (dma_resv_trylock(bo->tbo.base.resv)) {
- unsigned int domain;
- domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
- switch (domain) {
- case AMDGPU_GEM_DOMAIN_VRAM:
+ switch (bo->tbo.resource->mem_type) {
+ case TTM_PL_VRAM:
if (amdgpu_bo_in_cpu_visible_vram(bo))
placement = "VRAM VISIBLE";
else
placement = "VRAM";
break;
- case AMDGPU_GEM_DOMAIN_GTT:
+ case TTM_PL_TT:
+ case AMDGPU_PL_PREEMPT:
placement = "GTT";
break;
- case AMDGPU_GEM_DOMAIN_CPU:
+ case
