On 15/02/2023 14:36, Christian König wrote:
Am 15.02.23 um 14:32 schrieb Shashank Sharma:
On 15/02/2023 07:17, Christian König wrote:
Am 14.02.23 um 20:24 schrieb Shashank Sharma:
On 14/02/2023 19:31, Christian König wrote:
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
From: Alex Deucher <alexander.deucher@xxxxxxx>
This patch adds changes:
- to accommodate the new GEM domain DOORBELL
- to accommodate the new TTM PL DOORBELL
to manage doorbell allocations as GEM Objects.
V2: Addressed reviwe comments from Christian
- drop the doorbell changes for pinning/unpinning
- drop the doorbell changes for dma-buf map
- drop the doorbell changes for sgt
- no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index b48c9fd60c43..ff9979fecfd2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct
amdgpu_bo *abo, u32 domain)
c++;
}
+ if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
+ places[c].fpfn = 0;
+ places[c].lpfn = 0;
+ places[c].mem_type = AMDGPU_PL_DOORBELL;
+ places[c].flags = 0;
+ c++;
+ }
+
Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
I think the answer is *no* since mixing DOORBELL with CPU, GTT or
VRAM placement doesn't make sense, but do we enforce that somewhere?
I am not sure why do we need that ?
Userspace could otherwise specify DOORBEEL|CPU|GTT|VRAM as placement
which would overrun the array and be illegal.
Now when I understand this, how can we enforce this ? A size check
that blocks places to go over a certain value, which is fixed for
boorbell ?
In amdgpu_bo_create() we should have a check that if GDS, GWS, OA and
DOORBELL are specified they are specified all alone. In other words
those domains can't be mixed with anything else.
Christian.
Got it, let me check this out.
- Shashank
if (domain & AMDGPU_GEM_DOMAIN_GTT) {
places[c].fpfn = 0;
places[c].lpfn = 0;
@@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct
amdgpu_device *adev,
goto fail;
}
- /* TODO add more domains checks, such as
AMDGPU_GEM_DOMAIN_CPU */
+ /* TODO add more domains checks, such as
AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
Should we enforce that user space can only allocate 1 page doorbells?
Should we add a per-PID basis check ?
No, just a check that the allocation size of the doorbell BOs is
just 1 page.
Noted
- Shashank
Christian.
- Shashank
return true;
fail:
@@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
} else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
}
+
Unrelated change.
Noted
- Shashank
Regards,
Christian.
}
static const char *amdgpu_vram_names[] = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0e8f580769ab..e9dc24191fc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct
ttm_buffer_object *bo,
case AMDGPU_PL_GDS:
case AMDGPU_PL_GWS:
case AMDGPU_PL_OA:
+ case AMDGPU_PL_DOORBELL:
placement->num_placement = 0;
placement->num_busy_placement = 0;
return;
@@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct
ttm_buffer_object *bo, bool evict,
if (old_mem->mem_type == AMDGPU_PL_GDS ||
old_mem->mem_type == AMDGPU_PL_GWS ||
old_mem->mem_type == AMDGPU_PL_OA ||
+ old_mem->mem_type == AMDGPU_PL_DOORBELL ||
new_mem->mem_type == AMDGPU_PL_GDS ||
new_mem->mem_type == AMDGPU_PL_GWS ||
- new_mem->mem_type == AMDGPU_PL_OA) {
+ new_mem->mem_type == AMDGPU_PL_OA ||
+ new_mem->mem_type == AMDGPU_PL_DOORBELL) {
/* Nothing to save here */
ttm_bo_move_null(bo, new_mem);
goto out;
@@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct
ttm_device *bdev,
mem->bus.offset += adev->gmc.vram_aper_base;
mem->bus.is_iomem = true;
break;
+ case AMDGPU_PL_DOORBELL:
+ mem->bus.offset = mem->start << PAGE_SHIFT;
+ mem->bus.offset += adev->doorbell.doorbell_aper_base;
+ mem->bus.is_iomem = true;
+ break;
default:
return -EINVAL;
}
@@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct
ttm_tt *ttm, struct ttm_resource *mem)
flags |= AMDGPU_PTE_VALID;
if (mem && (mem->mem_type == TTM_PL_TT ||
+ mem->mem_type == AMDGPU_PL_DOORBELL ||
mem->mem_type == AMDGPU_PL_PREEMPT)) {
flags |= AMDGPU_PTE_SYSTEM;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 967b265dbfa1..9cf5d8419965 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -33,6 +33,7 @@
#define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
#define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
#define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
+#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
#define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
#define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
