* [PATCH 1/3] drm/amdkfd: Move a constant definition around
@ 2019-01-31 22:27 Zhao, Yong
[not found] ` <20190131222703.3591-1-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Zhao, Yong @ 2019-01-31 22:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
The similar definitions should be consecutive.
Change-Id: I936cf076363e641c60e0704d8405ae9493718e18
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 12b66330fc6d..e5ebcca7f031 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -97,17 +97,18 @@
#define KFD_CWSR_TBA_TMA_SIZE (PAGE_SIZE * 2)
#define KFD_CWSR_TMA_OFFSET PAGE_SIZE
+#define KFD_MAX_NUM_OF_QUEUES_PER_DEVICE \
+ (KFD_MAX_NUM_OF_PROCESSES * \
+ KFD_MAX_NUM_OF_QUEUES_PER_PROCESS)
+
+#define KFD_KERNEL_QUEUE_SIZE 2048
+
/*
* Kernel module parameter to specify maximum number of supported queues per
* device
*/
extern int max_num_of_queues_per_device;
-#define KFD_MAX_NUM_OF_QUEUES_PER_DEVICE \
- (KFD_MAX_NUM_OF_PROCESSES * \
- KFD_MAX_NUM_OF_QUEUES_PER_PROCESS)
-
-#define KFD_KERNEL_QUEUE_SIZE 2048
/* Kernel module parameter to specify the scheduling policy */
extern int sched_policy;
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <20190131222703.3591-1-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
@ 2019-01-31 22:27 ` Zhao, Yong
[not found] ` <20190131222703.3591-2-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-01-31 22:27 ` [PATCH 3/3] drm/amdkfd: Optimize out sdma doorbell array in kgd2kfd_shared_resources Zhao, Yong
1 sibling, 1 reply; 12+ messages in thread
From: Zhao, Yong @ 2019-01-31 22:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
Reserved doorbells for SDMA IH and VCN were not properly masked out
when allocating doorbells for CP user queues. This patch fixed that.
Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25 +++++++++++++------
.../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
7 files changed, 56 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index e957e42c539a..13710f34191a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
gpu_resources.sdma_doorbell[1][i+1] =
adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
}
- /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
- * SDMA, IH and VCN. So don't use them for the CP.
+
+ /* Because of the setting in registers like
+ * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
+ * lower 12 bits of doorbell address for routing, in
+ * order to route the CP queue doorbells to CP engine,
+ * the doorbells allocated to CP queues have to be
+ * outside the range set for SDMA, VCN, and IH blocks
+ * Prior to SOC15, all queues use queue ID to
+ * determine doorbells.
*/
- gpu_resources.reserved_doorbell_mask = 0x1e0;
- gpu_resources.reserved_doorbell_val = 0x0e0;
+ gpu_resources.reserved_doorbells_start =
+ adev->doorbell_index.sdma_engine[0];
+ gpu_resources.reserved_doorbells_end =
+ adev->doorbell_index.last_non_cp;
kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
index 1cfec06f81d4..4de431f7f380 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
@@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
uint32_t vce_ring6_7;
} uvd_vce;
};
+ uint32_t last_non_cp;
uint32_t max_assignment;
/* Per engine SDMA doorbell size in dword */
uint32_t sdma_doorbell_range;
@@ -143,6 +144,7 @@ typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
+ AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
} AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
@@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
+ AMDGPU_DOORBELL64_LAST_NON_CP = AMDGPU_DOORBELL64_VCE_RING6_7,
+
AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
AMDGPU_DOORBELL64_INVALID = 0xFFFF
} AMDGPU_DOORBELL64_ASSIGNMENT;
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
index 4b5d60ea3e78..fa0433199215 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
@@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct amdgpu_device *adev)
adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_DOORBELL64_VCE_RING2_3;
adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_DOORBELL64_VCE_RING4_5;
adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_DOORBELL64_VCE_RING6_7;
+
+ adev->doorbell_index.last_non_cp = AMDGPU_DOORBELL64_LAST_NON_CP;
+
/* In unit of dword doorbell */
adev->doorbell_index.max_assignment = AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
adev->doorbell_index.sdma_doorbell_range = 4;
diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
index 53716c593b2b..b1052caaff5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
@@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct amdgpu_device *adev)
adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
+
+ adev->doorbell_index.last_non_cp = AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
+
adev->doorbell_index.max_assignment = AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
adev->doorbell_index.sdma_doorbell_range = 20;
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index e5ebcca7f031..6b8459f852cc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -103,6 +103,15 @@
#define KFD_KERNEL_QUEUE_SIZE 2048
+/* 512 = 0x200
+ * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1) in the
+ * same SDMA engine, where i is a even number.
+ * For 8-bytes doorbells, it ensures that the mirror doorbell range (in terms
+ * of low 12 bit address for each HW engine) on the second doorbell page is
+ * the same as the range of the first doorbell page.*/
+#define KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
+
+
/*
* Kernel module parameter to specify maximum number of supported queues per
* device
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 80b36e860a0a..c7b67c65d62d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct qcm_process_device *qpd,
if (!qpd->doorbell_bitmap)
return -ENOMEM;
- /* Mask out any reserved doorbells */
- for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
- if ((dev->shared_resources.reserved_doorbell_mask & i) ==
- dev->shared_resources.reserved_doorbell_val) {
- set_bit(i, qpd->doorbell_bitmap);
- pr_debug("reserved doorbell 0x%03x\n", i);
- }
+ /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
+ * Because of the setting in registers like SDMA0_DOORBELL_RANGE etc.,
+ * BIF statically uses the lower 12 bits of doorbell address for
+ * routing. In order to route the CP queue doorbells to CP engine,
+ * the doorbells allocated to CP queues have to be outside the range
+ * set for SDMA, VCN, and IH blocks.
+ * Prior to SOC15, all queues use queue ID to
+ * determine doorbells. */
+ i = dev->shared_resources.reserved_doorbells_start;
+ while (i <= dev->shared_resources.reserved_doorbells_end) {
+ set_bit(i, qpd->doorbell_bitmap);
+ set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
+ qpd->doorbell_bitmap);
+ pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
+ i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
+
+ i++;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index 83d960110d23..b1bf45419d93 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
/* Bit n == 1 means Queue n is available for KFD */
DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
- /* Doorbell assignments (SOC15 and later chips only). Only
- * specific doorbells are routed to each SDMA engine. Others
- * are routed to IH and VCN. They are not usable by the CP.
- *
- * Any doorbell number D that satisfies the following condition
- * is reserved: (D & reserved_doorbell_mask) == reserved_doorbell_val
- *
- * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
- * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that means
- * mask would be set to 0x1e0 and val set to 0x0e0.
- */
unsigned int sdma_doorbell[2][8];
- unsigned int reserved_doorbell_mask;
- unsigned int reserved_doorbell_val;
+
+ /* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
+ * and VCN
+ */
+ unsigned int reserved_doorbells_start;
+ unsigned int reserved_doorbells_end;
/* Base address of doorbell aperture. */
phys_addr_t doorbell_physical_address;
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/amdkfd: Optimize out sdma doorbell array in kgd2kfd_shared_resources
[not found] ` <20190131222703.3591-1-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-01-31 22:27 ` [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15 Zhao, Yong
@ 2019-01-31 22:27 ` Zhao, Yong
[not found] ` <20190131222703.3591-3-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
1 sibling, 1 reply; 12+ messages in thread
From: Zhao, Yong @ 2019-01-31 22:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
We can directly calculate the sdma doorbell index in the process doorbell
pages through the doorbell_index structure in amdgpu_device, so no need
to cache them in kgd2kfd_shared_resources any more, resulting in more
portable code.
Change-Id: Ic657799856ed0256f36b01e502ef0cab263b1f49
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 55 ++++++-------------
.../drm/amd/amdkfd/kfd_device_queue_manager.c | 18 ++++--
.../gpu/drm/amd/include/kgd_kfd_interface.h | 2 +-
3 files changed, 31 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 13710f34191a..f050adc3f5da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -131,7 +131,7 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
{
- int i, n;
+ int i;
int last_valid_bit;
if (adev->kfd.dev) {
@@ -142,7 +142,9 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
.gpuvm_size = min(adev->vm_manager.max_pfn
<< AMDGPU_GPU_PAGE_SHIFT,
AMDGPU_GMC_HOLE_START),
- .drm_render_minor = adev->ddev->render->index
+ .drm_render_minor = adev->ddev->render->index,
+ .sdma_doorbell_idx = adev->doorbell_index.sdma_engine,
+
};
/* this is going to have a few of the MSBs set that we need to
@@ -172,45 +174,22 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
&gpu_resources.doorbell_aperture_size,
&gpu_resources.doorbell_start_offset);
- if (adev->asic_type < CHIP_VEGA10) {
- kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
- return;
- }
-
- n = (adev->asic_type < CHIP_VEGA20) ? 2 : 8;
-
- for (i = 0; i < n; i += 2) {
- /* On SOC15 the BIF is involved in routing
- * doorbells using the low 12 bits of the
- * address. Communicate the assignments to
- * KFD. KFD uses two doorbell pages per
- * process in case of 64-bit doorbells so we
- * can use each doorbell assignment twice.
+ if (adev->asic_type >= CHIP_VEGA10) {
+ /* Because of the setting in registers like
+ * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
+ * lower 12 bits of doorbell address for routing, in
+ * order to route the CP queue doorbells to CP engine,
+ * the doorbells allocated to CP queues have to be
+ * outside the range set for SDMA, VCN, and IH blocks
+ * Prior to SOC15, all queues use queue ID to
+ * determine doorbells.
*/
- gpu_resources.sdma_doorbell[0][i] =
- adev->doorbell_index.sdma_engine[0] + (i >> 1);
- gpu_resources.sdma_doorbell[0][i+1] =
- adev->doorbell_index.sdma_engine[0] + 0x200 + (i >> 1);
- gpu_resources.sdma_doorbell[1][i] =
- adev->doorbell_index.sdma_engine[1] + (i >> 1);
- gpu_resources.sdma_doorbell[1][i+1] =
- adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
+ gpu_resources.reserved_doorbells_start =
+ adev->doorbell_index.sdma_engine[0];
+ gpu_resources.reserved_doorbells_end =
+ adev->doorbell_index.last_non_cp;
}
- /* Because of the setting in registers like
- * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
- * lower 12 bits of doorbell address for routing, in
- * order to route the CP queue doorbells to CP engine,
- * the doorbells allocated to CP queues have to be
- * outside the range set for SDMA, VCN, and IH blocks
- * Prior to SOC15, all queues use queue ID to
- * determine doorbells.
- */
- gpu_resources.reserved_doorbells_start =
- adev->doorbell_index.sdma_engine[0];
- gpu_resources.reserved_doorbells_end =
- adev->doorbell_index.last_non_cp;
-
kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
}
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 8372556b52eb..81280ce5aa27 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -134,12 +134,20 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
*/
q->doorbell_id = q->properties.queue_id;
} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
- /* For SDMA queues on SOC15, use static doorbell
- * assignments based on the engine and queue.
+ /* For SDMA queues on SOC15 with 8-byte doorbell, use static
+ * doorbell assignments based on the engine and queue id.
+ * The doobell index distance between RLC (2*i) and (2*i+1)
+ * for a SDMA engine is 512.
+ * 512 8-byte doorbell distance (i.e. one page away) ensures
+ * that SDMA RLC (2*i+1) doorbell lies exactly in the doorbell
+ * OFFSET and SIZE set in register BIF_SDMA0_DOORBELL_RANGE.
*/
- q->doorbell_id = dev->shared_resources.sdma_doorbell
- [q->properties.sdma_engine_id]
- [q->properties.sdma_queue_id];
+ unsigned int *idx_offset =
+ dev->shared_resources.sdma_doorbell_idx;
+ q->doorbell_id = idx_offset[q->properties.sdma_engine_id]
+ + (q->properties.sdma_queue_id >> 1)
+ + (q->properties.sdma_queue_id % 2)
+ * KFD_QUEUE_DOORBELL_MIRROR_OFFSET;
} else {
/* For CP queues on SOC15 reserve a free doorbell ID */
unsigned int found;
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index b1bf45419d93..3559170f6fb3 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -137,7 +137,7 @@ struct kgd2kfd_shared_resources {
/* Bit n == 1 means Queue n is available for KFD */
DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
- unsigned int sdma_doorbell[2][8];
+ unsigned int *sdma_doorbell_idx;
/* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
* and VCN
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <20190131222703.3591-2-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
@ 2019-02-01 16:58 ` Zeng, Oak
2019-02-01 20:03 ` Kuehling, Felix
1 sibling, 0 replies; 12+ messages in thread
From: Zeng, Oak @ 2019-02-01 16:58 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
Can we delete userqueue_start/end? I think the original purpose for those two fields are to specify a doorbell range for user queues, but were never used. Now we clearly want user queue to use all the range, minus range of IH/MM/SDMA. So those two fields will not be used.
Thanks,
Oak
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Zhao, Yong
Sent: Thursday, January 31, 2019 5:27 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong <Yong.Zhao@amd.com>
Subject: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
Reserved doorbells for SDMA IH and VCN were not properly masked out when allocating doorbells for CP user queues. This patch fixed that.
Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++ drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++ drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25 +++++++++++++------
.../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
7 files changed, 56 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index e957e42c539a..13710f34191a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
gpu_resources.sdma_doorbell[1][i+1] =
adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
}
- /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
- * SDMA, IH and VCN. So don't use them for the CP.
+
+ /* Because of the setting in registers like
+ * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
+ * lower 12 bits of doorbell address for routing, in
+ * order to route the CP queue doorbells to CP engine,
+ * the doorbells allocated to CP queues have to be
+ * outside the range set for SDMA, VCN, and IH blocks
+ * Prior to SOC15, all queues use queue ID to
+ * determine doorbells.
*/
- gpu_resources.reserved_doorbell_mask = 0x1e0;
- gpu_resources.reserved_doorbell_val = 0x0e0;
+ gpu_resources.reserved_doorbells_start =
+ adev->doorbell_index.sdma_engine[0];
+ gpu_resources.reserved_doorbells_end =
+ adev->doorbell_index.last_non_cp;
kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
index 1cfec06f81d4..4de431f7f380 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
@@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
uint32_t vce_ring6_7;
} uvd_vce;
};
+ uint32_t last_non_cp;
uint32_t max_assignment;
/* Per engine SDMA doorbell size in dword */
uint32_t sdma_doorbell_range;
@@ -143,6 +144,7 @@ typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
+ AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
} AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
@@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
+ AMDGPU_DOORBELL64_LAST_NON_CP = AMDGPU_DOORBELL64_VCE_RING6_7,
+
AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
AMDGPU_DOORBELL64_INVALID = 0xFFFF
} AMDGPU_DOORBELL64_ASSIGNMENT;
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
index 4b5d60ea3e78..fa0433199215 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
@@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct amdgpu_device *adev)
adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_DOORBELL64_VCE_RING2_3;
adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_DOORBELL64_VCE_RING4_5;
adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_DOORBELL64_VCE_RING6_7;
+
+ adev->doorbell_index.last_non_cp = AMDGPU_DOORBELL64_LAST_NON_CP;
+
/* In unit of dword doorbell */
adev->doorbell_index.max_assignment = AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
adev->doorbell_index.sdma_doorbell_range = 4; diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
index 53716c593b2b..b1052caaff5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
@@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct amdgpu_device *adev)
adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
+
+ adev->doorbell_index.last_non_cp =
+AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
+
adev->doorbell_index.max_assignment = AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
adev->doorbell_index.sdma_doorbell_range = 20; } diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index e5ebcca7f031..6b8459f852cc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -103,6 +103,15 @@
#define KFD_KERNEL_QUEUE_SIZE 2048
+/* 512 = 0x200
+ * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1) in
+the
+ * same SDMA engine, where i is a even number.
+ * For 8-bytes doorbells, it ensures that the mirror doorbell range (in
+terms
+ * of low 12 bit address for each HW engine) on the second doorbell
+page is
+ * the same as the range of the first doorbell page.*/ #define
+KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
+
+
/*
* Kernel module parameter to specify maximum number of supported queues per
* device
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 80b36e860a0a..c7b67c65d62d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct qcm_process_device *qpd,
if (!qpd->doorbell_bitmap)
return -ENOMEM;
- /* Mask out any reserved doorbells */
- for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
- if ((dev->shared_resources.reserved_doorbell_mask & i) ==
- dev->shared_resources.reserved_doorbell_val) {
- set_bit(i, qpd->doorbell_bitmap);
- pr_debug("reserved doorbell 0x%03x\n", i);
- }
+ /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
+ * Because of the setting in registers like SDMA0_DOORBELL_RANGE etc.,
+ * BIF statically uses the lower 12 bits of doorbell address for
+ * routing. In order to route the CP queue doorbells to CP engine,
+ * the doorbells allocated to CP queues have to be outside the range
+ * set for SDMA, VCN, and IH blocks.
+ * Prior to SOC15, all queues use queue ID to
+ * determine doorbells. */
+ i = dev->shared_resources.reserved_doorbells_start;
+ while (i <= dev->shared_resources.reserved_doorbells_end) {
+ set_bit(i, qpd->doorbell_bitmap);
+ set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
+ qpd->doorbell_bitmap);
+ pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
+ i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
+
+ i++;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index 83d960110d23..b1bf45419d93 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
/* Bit n == 1 means Queue n is available for KFD */
DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
- /* Doorbell assignments (SOC15 and later chips only). Only
- * specific doorbells are routed to each SDMA engine. Others
- * are routed to IH and VCN. They are not usable by the CP.
- *
- * Any doorbell number D that satisfies the following condition
- * is reserved: (D & reserved_doorbell_mask) == reserved_doorbell_val
- *
- * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
- * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that means
- * mask would be set to 0x1e0 and val set to 0x0e0.
- */
unsigned int sdma_doorbell[2][8];
- unsigned int reserved_doorbell_mask;
- unsigned int reserved_doorbell_val;
+
+ /* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
+ * and VCN
+ */
+ unsigned int reserved_doorbells_start;
+ unsigned int reserved_doorbells_end;
/* Base address of doorbell aperture. */
phys_addr_t doorbell_physical_address;
--
2.17.1
_______________________________________________
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 related [flat|nested] 12+ messages in thread
* RE: [PATCH 3/3] drm/amdkfd: Optimize out sdma doorbell array in kgd2kfd_shared_resources
[not found] ` <20190131222703.3591-3-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
@ 2019-02-01 17:09 ` Zeng, Oak
0 siblings, 0 replies; 12+ messages in thread
From: Zeng, Oak @ 2019-02-01 17:09 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
Reviewed-by: Oak Zeng <Oak.Zeng@amd.com>. Patch looks good to me. It is better Felix to double confirm.
Thanks,
Oak
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Zhao, Yong
Sent: Thursday, January 31, 2019 5:27 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong <Yong.Zhao@amd.com>
Subject: [PATCH 3/3] drm/amdkfd: Optimize out sdma doorbell array in kgd2kfd_shared_resources
We can directly calculate the sdma doorbell index in the process doorbell pages through the doorbell_index structure in amdgpu_device, so no need to cache them in kgd2kfd_shared_resources any more, resulting in more portable code.
Change-Id: Ic657799856ed0256f36b01e502ef0cab263b1f49
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 55 ++++++-------------
.../drm/amd/amdkfd/kfd_device_queue_manager.c | 18 ++++--
.../gpu/drm/amd/include/kgd_kfd_interface.h | 2 +-
3 files changed, 31 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 13710f34191a..f050adc3f5da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -131,7 +131,7 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
void amdgpu_amdkfd_device_init(struct amdgpu_device *adev) {
- int i, n;
+ int i;
int last_valid_bit;
if (adev->kfd.dev) {
@@ -142,7 +142,9 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
.gpuvm_size = min(adev->vm_manager.max_pfn
<< AMDGPU_GPU_PAGE_SHIFT,
AMDGPU_GMC_HOLE_START),
- .drm_render_minor = adev->ddev->render->index
+ .drm_render_minor = adev->ddev->render->index,
+ .sdma_doorbell_idx = adev->doorbell_index.sdma_engine,
+
};
/* this is going to have a few of the MSBs set that we need to @@ -172,45 +174,22 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
&gpu_resources.doorbell_aperture_size,
&gpu_resources.doorbell_start_offset);
- if (adev->asic_type < CHIP_VEGA10) {
- kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
- return;
- }
-
- n = (adev->asic_type < CHIP_VEGA20) ? 2 : 8;
-
- for (i = 0; i < n; i += 2) {
- /* On SOC15 the BIF is involved in routing
- * doorbells using the low 12 bits of the
- * address. Communicate the assignments to
- * KFD. KFD uses two doorbell pages per
- * process in case of 64-bit doorbells so we
- * can use each doorbell assignment twice.
+ if (adev->asic_type >= CHIP_VEGA10) {
+ /* Because of the setting in registers like
+ * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
+ * lower 12 bits of doorbell address for routing, in
+ * order to route the CP queue doorbells to CP engine,
+ * the doorbells allocated to CP queues have to be
+ * outside the range set for SDMA, VCN, and IH blocks
+ * Prior to SOC15, all queues use queue ID to
+ * determine doorbells.
*/
- gpu_resources.sdma_doorbell[0][i] =
- adev->doorbell_index.sdma_engine[0] + (i >> 1);
- gpu_resources.sdma_doorbell[0][i+1] =
- adev->doorbell_index.sdma_engine[0] + 0x200 + (i >> 1);
- gpu_resources.sdma_doorbell[1][i] =
- adev->doorbell_index.sdma_engine[1] + (i >> 1);
- gpu_resources.sdma_doorbell[1][i+1] =
- adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
+ gpu_resources.reserved_doorbells_start =
+ adev->doorbell_index.sdma_engine[0];
+ gpu_resources.reserved_doorbells_end =
+ adev->doorbell_index.last_non_cp;
}
- /* Because of the setting in registers like
- * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
- * lower 12 bits of doorbell address for routing, in
- * order to route the CP queue doorbells to CP engine,
- * the doorbells allocated to CP queues have to be
- * outside the range set for SDMA, VCN, and IH blocks
- * Prior to SOC15, all queues use queue ID to
- * determine doorbells.
- */
- gpu_resources.reserved_doorbells_start =
- adev->doorbell_index.sdma_engine[0];
- gpu_resources.reserved_doorbells_end =
- adev->doorbell_index.last_non_cp;
-
kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
}
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 8372556b52eb..81280ce5aa27 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -134,12 +134,20 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
*/
q->doorbell_id = q->properties.queue_id;
} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
- /* For SDMA queues on SOC15, use static doorbell
- * assignments based on the engine and queue.
+ /* For SDMA queues on SOC15 with 8-byte doorbell, use static
+ * doorbell assignments based on the engine and queue id.
+ * The doobell index distance between RLC (2*i) and (2*i+1)
+ * for a SDMA engine is 512.
+ * 512 8-byte doorbell distance (i.e. one page away) ensures
+ * that SDMA RLC (2*i+1) doorbell lies exactly in the doorbell
+ * OFFSET and SIZE set in register BIF_SDMA0_DOORBELL_RANGE.
*/
- q->doorbell_id = dev->shared_resources.sdma_doorbell
- [q->properties.sdma_engine_id]
- [q->properties.sdma_queue_id];
+ unsigned int *idx_offset =
+ dev->shared_resources.sdma_doorbell_idx;
+ q->doorbell_id = idx_offset[q->properties.sdma_engine_id]
+ + (q->properties.sdma_queue_id >> 1)
+ + (q->properties.sdma_queue_id % 2)
+ * KFD_QUEUE_DOORBELL_MIRROR_OFFSET;
} else {
/* For CP queues on SOC15 reserve a free doorbell ID */
unsigned int found;
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index b1bf45419d93..3559170f6fb3 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -137,7 +137,7 @@ struct kgd2kfd_shared_resources {
/* Bit n == 1 means Queue n is available for KFD */
DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
- unsigned int sdma_doorbell[2][8];
+ unsigned int *sdma_doorbell_idx;
/* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
* and VCN
--
2.17.1
_______________________________________________
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 related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <20190131222703.3591-2-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-02-01 16:58 ` Zeng, Oak
@ 2019-02-01 20:03 ` Kuehling, Felix
[not found] ` <e01f29c1-74e8-4dd6-81fc-b25c0432798f-5C7GfCeVMHo@public.gmane.org>
1 sibling, 1 reply; 12+ messages in thread
From: Kuehling, Felix @ 2019-02-01 20:03 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> Reserved doorbells for SDMA IH and VCN were not properly masked out
> when allocating doorbells for CP user queues. This patch fixed that.
>
> Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
One mostly cosmetic comment inline. With that fixed, the series is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25 +++++++++++++------
> .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> 7 files changed, 56 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index e957e42c539a..13710f34191a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
> gpu_resources.sdma_doorbell[1][i+1] =
> adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> }
> - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> - * SDMA, IH and VCN. So don't use them for the CP.
> +
> + /* Because of the setting in registers like
> + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> + * lower 12 bits of doorbell address for routing, in
> + * order to route the CP queue doorbells to CP engine,
> + * the doorbells allocated to CP queues have to be
> + * outside the range set for SDMA, VCN, and IH blocks
> + * Prior to SOC15, all queues use queue ID to
> + * determine doorbells.
> */
> - gpu_resources.reserved_doorbell_mask = 0x1e0;
> - gpu_resources.reserved_doorbell_val = 0x0e0;
> + gpu_resources.reserved_doorbells_start =
> + adev->doorbell_index.sdma_engine[0];
> + gpu_resources.reserved_doorbells_end =
> + adev->doorbell_index.last_non_cp;
>
> kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 1cfec06f81d4..4de431f7f380 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> uint32_t vce_ring6_7;
> } uvd_vce;
> };
> + uint32_t last_non_cp;
> uint32_t max_assignment;
> /* Per engine SDMA doorbell size in dword */
> uint32_t sdma_doorbell_range;
> @@ -143,6 +144,7 @@ typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
>
> + AMDGPU_DOORBELL64_LAST_NON_CP = AMDGPU_DOORBELL64_VCE_RING6_7,
> +
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> AMDGPU_DOORBELL64_INVALID = 0xFFFF
> } AMDGPU_DOORBELL64_ASSIGNMENT;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> index 4b5d60ea3e78..fa0433199215 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct amdgpu_device *adev)
> adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_DOORBELL64_VCE_RING2_3;
> adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_DOORBELL64_VCE_RING4_5;
> adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_DOORBELL64_VCE_RING6_7;
> +
> + adev->doorbell_index.last_non_cp = AMDGPU_DOORBELL64_LAST_NON_CP;
> +
> /* In unit of dword doorbell */
> adev->doorbell_index.max_assignment = AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> adev->doorbell_index.sdma_doorbell_range = 4;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> index 53716c593b2b..b1052caaff5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct amdgpu_device *adev)
> adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> +
> + adev->doorbell_index.last_non_cp = AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> +
> adev->doorbell_index.max_assignment = AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> adev->doorbell_index.sdma_doorbell_range = 20;
> }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index e5ebcca7f031..6b8459f852cc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -103,6 +103,15 @@
>
> #define KFD_KERNEL_QUEUE_SIZE 2048
>
> +/* 512 = 0x200
> + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1) in the
> + * same SDMA engine, where i is a even number.
> + * For 8-bytes doorbells, it ensures that the mirror doorbell range (in terms
> + * of low 12 bit address for each HW engine) on the second doorbell page is
> + * the same as the range of the first doorbell page.*/
> +#define KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> +
> +
> /*
> * Kernel module parameter to specify maximum number of supported queues per
> * device
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 80b36e860a0a..c7b67c65d62d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct qcm_process_device *qpd,
> if (!qpd->doorbell_bitmap)
> return -ENOMEM;
>
> - /* Mask out any reserved doorbells */
> - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> - if ((dev->shared_resources.reserved_doorbell_mask & i) ==
> - dev->shared_resources.reserved_doorbell_val) {
> - set_bit(i, qpd->doorbell_bitmap);
> - pr_debug("reserved doorbell 0x%03x\n", i);
> - }
> + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> + * Because of the setting in registers like SDMA0_DOORBELL_RANGE etc.,
> + * BIF statically uses the lower 12 bits of doorbell address for
> + * routing. In order to route the CP queue doorbells to CP engine,
> + * the doorbells allocated to CP queues have to be outside the range
> + * set for SDMA, VCN, and IH blocks.
> + * Prior to SOC15, all queues use queue ID to
> + * determine doorbells. */
> + i = dev->shared_resources.reserved_doorbells_start;
> + while (i <= dev->shared_resources.reserved_doorbells_end) {
This should be a for-loop to be more obvious:
for (i = dev->shared_resources.reserved_doorbells_start;
i <= dev->shared_resources.reserved_doorbells_end;
i++) {
...
}
> + set_bit(i, qpd->doorbell_bitmap);
> + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> + qpd->doorbell_bitmap);
> + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> +
> + i++;
> + }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index 83d960110d23..b1bf45419d93 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> /* Bit n == 1 means Queue n is available for KFD */
> DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
>
> - /* Doorbell assignments (SOC15 and later chips only). Only
> - * specific doorbells are routed to each SDMA engine. Others
> - * are routed to IH and VCN. They are not usable by the CP.
> - *
> - * Any doorbell number D that satisfies the following condition
> - * is reserved: (D & reserved_doorbell_mask) == reserved_doorbell_val
> - *
> - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that means
> - * mask would be set to 0x1e0 and val set to 0x0e0.
> - */
> unsigned int sdma_doorbell[2][8];
> - unsigned int reserved_doorbell_mask;
> - unsigned int reserved_doorbell_val;
> +
> + /* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
> + * and VCN
> + */
> + unsigned int reserved_doorbells_start;
> + unsigned int reserved_doorbells_end;
>
> /* Base address of doorbell aperture. */
> phys_addr_t doorbell_physical_address;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <e01f29c1-74e8-4dd6-81fc-b25c0432798f-5C7GfCeVMHo@public.gmane.org>
@ 2019-02-02 0:04 ` Zhao, Yong
[not found] ` <BL0PR12MB250046784085018A70E34EC0F0920-b4cIHhjg/p89/wkoUb0eEAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Zhao, Yong @ 2019-02-02 0:04 UTC (permalink / raw)
To: Kuehling, Felix, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Zeng, Oak
[-- Attachment #1.1: Type: text/plain, Size: 11451 bytes --]
Oak,
It is a good idea to remove the userqueue_start/end. However, when doing so, I came to the following code in gfx_v9_0_kiq_init_register() :
/* enable the doorbell if requested */
if (ring->use_doorbell) {
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
(adev->doorbell_index.kiq * 2) << 2);
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
(adev->doorbell_index.userqueue_end * 2) << 2);
}
I remember that you reached the conclusion in one email thread that mmCP_MEC_DOORBELL_RANGE_UPPER/LOWER did not have effect at all on gfx9. Have we got any firm conclusions from HW engineer in the end? If those two registers are still valid, we should enforce the range for CP queues.
Regards,
Yong
________________________________
From: Kuehling, Felix
Sent: Friday, February 1, 2019 3:03 PM
To: Zhao, Yong; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> Reserved doorbells for SDMA IH and VCN were not properly masked out
> when allocating doorbells for CP user queues. This patch fixed that.
>
> Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> Signed-off-by: Yong Zhao <Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
One mostly cosmetic comment inline. With that fixed, the series is
Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25 +++++++++++++------
> .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> 7 files changed, 56 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index e957e42c539a..13710f34191a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
> gpu_resources.sdma_doorbell[1][i+1] =
> adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> }
> - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> - * SDMA, IH and VCN. So don't use them for the CP.
> +
> + /* Because of the setting in registers like
> + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> + * lower 12 bits of doorbell address for routing, in
> + * order to route the CP queue doorbells to CP engine,
> + * the doorbells allocated to CP queues have to be
> + * outside the range set for SDMA, VCN, and IH blocks
> + * Prior to SOC15, all queues use queue ID to
> + * determine doorbells.
> */
> - gpu_resources.reserved_doorbell_mask = 0x1e0;
> - gpu_resources.reserved_doorbell_val = 0x0e0;
> + gpu_resources.reserved_doorbells_start =
> + adev->doorbell_index.sdma_engine[0];
> + gpu_resources.reserved_doorbells_end =
> + adev->doorbell_index.last_non_cp;
>
> kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 1cfec06f81d4..4de431f7f380 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> uint32_t vce_ring6_7;
> } uvd_vce;
> };
> + uint32_t last_non_cp;
> uint32_t max_assignment;
> /* Per engine SDMA doorbell size in dword */
> uint32_t sdma_doorbell_range;
> @@ -143,6 +144,7 @@ typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
>
> + AMDGPU_DOORBELL64_LAST_NON_CP = AMDGPU_DOORBELL64_VCE_RING6_7,
> +
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> AMDGPU_DOORBELL64_INVALID = 0xFFFF
> } AMDGPU_DOORBELL64_ASSIGNMENT;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> index 4b5d60ea3e78..fa0433199215 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct amdgpu_device *adev)
> adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_DOORBELL64_VCE_RING2_3;
> adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_DOORBELL64_VCE_RING4_5;
> adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_DOORBELL64_VCE_RING6_7;
> +
> + adev->doorbell_index.last_non_cp = AMDGPU_DOORBELL64_LAST_NON_CP;
> +
> /* In unit of dword doorbell */
> adev->doorbell_index.max_assignment = AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> adev->doorbell_index.sdma_doorbell_range = 4;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> index 53716c593b2b..b1052caaff5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct amdgpu_device *adev)
> adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> +
> + adev->doorbell_index.last_non_cp = AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> +
> adev->doorbell_index.max_assignment = AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> adev->doorbell_index.sdma_doorbell_range = 20;
> }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index e5ebcca7f031..6b8459f852cc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -103,6 +103,15 @@
>
> #define KFD_KERNEL_QUEUE_SIZE 2048
>
> +/* 512 = 0x200
> + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1) in the
> + * same SDMA engine, where i is a even number.
> + * For 8-bytes doorbells, it ensures that the mirror doorbell range (in terms
> + * of low 12 bit address for each HW engine) on the second doorbell page is
> + * the same as the range of the first doorbell page.*/
> +#define KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> +
> +
> /*
> * Kernel module parameter to specify maximum number of supported queues per
> * device
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 80b36e860a0a..c7b67c65d62d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct qcm_process_device *qpd,
> if (!qpd->doorbell_bitmap)
> return -ENOMEM;
>
> - /* Mask out any reserved doorbells */
> - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> - if ((dev->shared_resources.reserved_doorbell_mask & i) ==
> - dev->shared_resources.reserved_doorbell_val) {
> - set_bit(i, qpd->doorbell_bitmap);
> - pr_debug("reserved doorbell 0x%03x\n", i);
> - }
> + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> + * Because of the setting in registers like SDMA0_DOORBELL_RANGE etc.,
> + * BIF statically uses the lower 12 bits of doorbell address for
> + * routing. In order to route the CP queue doorbells to CP engine,
> + * the doorbells allocated to CP queues have to be outside the range
> + * set for SDMA, VCN, and IH blocks.
> + * Prior to SOC15, all queues use queue ID to
> + * determine doorbells. */
> + i = dev->shared_resources.reserved_doorbells_start;
> + while (i <= dev->shared_resources.reserved_doorbells_end) {
This should be a for-loop to be more obvious:
for (i = dev->shared_resources.reserved_doorbells_start;
i <= dev->shared_resources.reserved_doorbells_end;
i++) {
...
}
> + set_bit(i, qpd->doorbell_bitmap);
> + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> + qpd->doorbell_bitmap);
> + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> +
> + i++;
> + }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index 83d960110d23..b1bf45419d93 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> /* Bit n == 1 means Queue n is available for KFD */
> DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
>
> - /* Doorbell assignments (SOC15 and later chips only). Only
> - * specific doorbells are routed to each SDMA engine. Others
> - * are routed to IH and VCN. They are not usable by the CP.
> - *
> - * Any doorbell number D that satisfies the following condition
> - * is reserved: (D & reserved_doorbell_mask) == reserved_doorbell_val
> - *
> - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that means
> - * mask would be set to 0x1e0 and val set to 0x0e0.
> - */
> unsigned int sdma_doorbell[2][8];
> - unsigned int reserved_doorbell_mask;
> - unsigned int reserved_doorbell_val;
> +
> + /* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
> + * and VCN
> + */
> + unsigned int reserved_doorbells_start;
> + unsigned int reserved_doorbells_end;
>
> /* Base address of doorbell aperture. */
> phys_addr_t doorbell_physical_address;
[-- Attachment #1.2: Type: text/html, Size: 22502 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <BL0PR12MB250046784085018A70E34EC0F0920-b4cIHhjg/p89/wkoUb0eEAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-02-02 15:01 ` Zeng, Oak
[not found] ` <DM5PR12MB25826F20FB8EF1E7E4B6348480930-2J9CzHegvk8nywYgqlzn1wdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Zeng, Oak @ 2019-02-02 15:01 UTC (permalink / raw)
To: Zhao, Yong, Kuehling, Felix, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 12232 bytes --]
Hi Yong,
mmCP_MEC_DOORBELL_RANGE_LOWER/UPPER are used at CP to double check a doorbell ringing routed to CP. It is a must to program. We have to keep userqueue_start/end. Sorry for the confusion.
Regards,
Oak
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> On Behalf Of Zhao, Yong
Sent: Friday, February 1, 2019 7:04 PM
To: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
Oak,
It is a good idea to remove the userqueue_start/end. However, when doing so, I came to the following code in gfx_v9_0_kiq_init_register() :
/* enable the doorbell if requested */
if (ring->use_doorbell) {
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
(adev->doorbell_index.kiq * 2) << 2);
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
(adev->doorbell_index.userqueue_end * 2) << 2);
}
I remember that you reached the conclusion in one email thread that mmCP_MEC_DOORBELL_RANGE_UPPER/LOWER did not have effect at all on gfx9. Have we got any firm conclusions from HW engineer in the end? If those two registers are still valid, we should enforce the range for CP queues.
Regards,
Yong
________________________________
From: Kuehling, Felix
Sent: Friday, February 1, 2019 3:03 PM
To: Zhao, Yong; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lzq7FAFJ/aeQ@public.gmane.orgtop.org>
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> Reserved doorbells for SDMA IH and VCN were not properly masked out
> when allocating doorbells for CP user queues. This patch fixed that.
>
> Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> Signed-off-by: Yong Zhao <Yong.Zhao-5C7GfCeVMHo@public.gmane.org<mailto:Yong.Zhao-5C7GfCeVMHo@public.gmane.org>>
One mostly cosmetic comment inline. With that fixed, the series is
Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org<mailto:Felix.Kuehling@amd.com>>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25 +++++++++++++------
> .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> 7 files changed, 56 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index e957e42c539a..13710f34191a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
> gpu_resources.sdma_doorbell[1][i+1] =
> adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> }
> - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> - * SDMA, IH and VCN. So don't use them for the CP.
> +
> + /* Because of the setting in registers like
> + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> + * lower 12 bits of doorbell address for routing, in
> + * order to route the CP queue doorbells to CP engine,
> + * the doorbells allocated to CP queues have to be
> + * outside the range set for SDMA, VCN, and IH blocks
> + * Prior to SOC15, all queues use queue ID to
> + * determine doorbells.
> */
> - gpu_resources.reserved_doorbell_mask = 0x1e0;
> - gpu_resources.reserved_doorbell_val = 0x0e0;
> + gpu_resources.reserved_doorbells_start =
> + adev->doorbell_index.sdma_engine[0];
> + gpu_resources.reserved_doorbells_end =
> + adev->doorbell_index.last_non_cp;
>
> kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 1cfec06f81d4..4de431f7f380 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> uint32_t vce_ring6_7;
> } uvd_vce;
> };
> + uint32_t last_non_cp;
> uint32_t max_assignment;
> /* Per engine SDMA doorbell size in dword */
> uint32_t sdma_doorbell_range;
> @@ -143,6 +144,7 @@ typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
>
> + AMDGPU_DOORBELL64_LAST_NON_CP = AMDGPU_DOORBELL64_VCE_RING6_7,
> +
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> AMDGPU_DOORBELL64_INVALID = 0xFFFF
> } AMDGPU_DOORBELL64_ASSIGNMENT;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> index 4b5d60ea3e78..fa0433199215 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct amdgpu_device *adev)
> adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_DOORBELL64_VCE_RING2_3;
> adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_DOORBELL64_VCE_RING4_5;
> adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_DOORBELL64_VCE_RING6_7;
> +
> + adev->doorbell_index.last_non_cp = AMDGPU_DOORBELL64_LAST_NON_CP;
> +
> /* In unit of dword doorbell */
> adev->doorbell_index.max_assignment = AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> adev->doorbell_index.sdma_doorbell_range = 4;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> index 53716c593b2b..b1052caaff5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct amdgpu_device *adev)
> adev->doorbell_index.uvd_vce.vce_ring2_3 = AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> +
> + adev->doorbell_index.last_non_cp = AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> +
> adev->doorbell_index.max_assignment = AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> adev->doorbell_index.sdma_doorbell_range = 20;
> }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index e5ebcca7f031..6b8459f852cc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -103,6 +103,15 @@
>
> #define KFD_KERNEL_QUEUE_SIZE 2048
>
> +/* 512 = 0x200
> + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1) in the
> + * same SDMA engine, where i is a even number.
> + * For 8-bytes doorbells, it ensures that the mirror doorbell range (in terms
> + * of low 12 bit address for each HW engine) on the second doorbell page is
> + * the same as the range of the first doorbell page.*/
> +#define KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> +
> +
> /*
> * Kernel module parameter to specify maximum number of supported queues per
> * device
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 80b36e860a0a..c7b67c65d62d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct qcm_process_device *qpd,
> if (!qpd->doorbell_bitmap)
> return -ENOMEM;
>
> - /* Mask out any reserved doorbells */
> - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> - if ((dev->shared_resources.reserved_doorbell_mask & i) ==
> - dev->shared_resources.reserved_doorbell_val) {
> - set_bit(i, qpd->doorbell_bitmap);
> - pr_debug("reserved doorbell 0x%03x\n", i);
> - }
> + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> + * Because of the setting in registers like SDMA0_DOORBELL_RANGE etc.,
> + * BIF statically uses the lower 12 bits of doorbell address for
> + * routing. In order to route the CP queue doorbells to CP engine,
> + * the doorbells allocated to CP queues have to be outside the range
> + * set for SDMA, VCN, and IH blocks.
> + * Prior to SOC15, all queues use queue ID to
> + * determine doorbells. */
> + i = dev->shared_resources.reserved_doorbells_start;
> + while (i <= dev->shared_resources.reserved_doorbells_end) {
This should be a for-loop to be more obvious:
for (i = dev->shared_resources.reserved_doorbells_start;
i <= dev->shared_resources.reserved_doorbells_end;
i++) {
...
}
> + set_bit(i, qpd->doorbell_bitmap);
> + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> + qpd->doorbell_bitmap);
> + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> +
> + i++;
> + }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index 83d960110d23..b1bf45419d93 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> /* Bit n == 1 means Queue n is available for KFD */
> DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
>
> - /* Doorbell assignments (SOC15 and later chips only). Only
> - * specific doorbells are routed to each SDMA engine. Others
> - * are routed to IH and VCN. They are not usable by the CP.
> - *
> - * Any doorbell number D that satisfies the following condition
> - * is reserved: (D & reserved_doorbell_mask) == reserved_doorbell_val
> - *
> - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that means
> - * mask would be set to 0x1e0 and val set to 0x0e0.
> - */
> unsigned int sdma_doorbell[2][8];
> - unsigned int reserved_doorbell_mask;
> - unsigned int reserved_doorbell_val;
> +
> + /* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
> + * and VCN
> + */
> + unsigned int reserved_doorbells_start;
> + unsigned int reserved_doorbells_end;
>
> /* Base address of doorbell aperture. */
> phys_addr_t doorbell_physical_address;
[-- Attachment #1.2: Type: text/html, Size: 25722 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <DM5PR12MB25826F20FB8EF1E7E4B6348480930-2J9CzHegvk8nywYgqlzn1wdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-02-04 16:34 ` Kuehling, Felix
[not found] ` <5aca7b73-2ef6-8a33-4a1c-3f876459de40-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Kuehling, Felix @ 2019-02-04 16:34 UTC (permalink / raw)
To: Zeng, Oak, Zhao, Yong, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
I don't see anything about userqueue start/end in Yong's patch. That
said, I think you're mixing up two different things. MEC_DOORBELL_RANGE
is not the same as userqueues? We have MEC kernel queues as well that
would have to fall in the same range. And we have SDMA user queues that
don't belong in the MEC doorbell range. Therefore I think the name
USERQUEUE_START/END is misleading.
Regards,
Felix
On 2019-02-02 10:01 a.m., Zeng, Oak wrote:
>
> Hi Yong,
>
> mmCP_MEC_DOORBELL_RANGE_LOWER/UPPER are used at CP to double check a
> doorbell ringing routed to CP. It is a must to program. We have to
> keep userqueue_start/end. Sorry for the confusion.
>
> Regards,
>
> Oak
>
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> *On Behalf Of
> *Zhao, Yong
> *Sent:* Friday, February 1, 2019 7:04 PM
> *To:* Kuehling, Felix <Felix.Kuehling@amd.com>;
> amd-gfx@lists.freedesktop.org; Zeng, Oak <Oak.Zeng@amd.com>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> Oak,
>
> It is a good idea to remove the userqueue_start/end. However, when
> doing so, I came to the following code in gfx_v9_0_kiq_init_register() :
>
> /* enable the doorbell if requested */
>
> if (ring->use_doorbell) {
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
>
> (adev->doorbell_index.kiq * 2) << 2);
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>
> (adev->doorbell_index.userqueue_end * 2) << 2);
>
> }
>
> I remember that you reached the conclusion in one email thread that
> mmCP_MEC_DOORBELL_RANGE_UPPER/LOWER did not have effect at all on
> gfx9. Have we got any firm conclusions from HW engineer in the end? If
> those two registers are still valid, we should enforce the range for
> CP queues.
>
> Regards,
>
> Yong
>
> ------------------------------------------------------------------------
>
> *From:*Kuehling, Felix
> *Sent:* Friday, February 1, 2019 3:03 PM
> *To:* Zhao, Yong; amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> > Reserved doorbells for SDMA IH and VCN were not properly masked out
> > when allocating doorbells for CP user queues. This patch fixed that.
> >
> > Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> > Signed-off-by: Yong Zhao <Yong.Zhao@amd.com <mailto:Yong.Zhao@amd.com>>
>
> One mostly cosmetic comment inline. With that fixed, the series is
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com
> <mailto:Felix.Kuehling@amd.com>>
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> > drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25 +++++++++++++------
> > .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> > 7 files changed, 56 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index e957e42c539a..13710f34191a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct
> amdgpu_device *adev)
> > gpu_resources.sdma_doorbell[1][i+1] =
> > adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> > }
> > - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> > - * SDMA, IH and VCN. So don't use them for the CP.
> > +
> > + /* Because of the setting in registers like
> > + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> > + * lower 12 bits of doorbell address for routing, in
> > + * order to route the CP queue doorbells to CP engine,
> > + * the doorbells allocated to CP queues have to be
> > + * outside the range set for SDMA, VCN, and IH blocks
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells.
> > */
> > - gpu_resources.reserved_doorbell_mask = 0x1e0;
> > - gpu_resources.reserved_doorbell_val = 0x0e0;
> > + gpu_resources.reserved_doorbells_start =
> > + adev->doorbell_index.sdma_engine[0];
> > + gpu_resources.reserved_doorbells_end =
> > + adev->doorbell_index.last_non_cp;
> >
> > kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > index 1cfec06f81d4..4de431f7f380 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> > uint32_t vce_ring6_7;
> > } uvd_vce;
> > };
> > + uint32_t last_non_cp;
> > uint32_t max_assignment;
> > /* Per engine SDMA doorbell size in dword */
> > uint32_t sdma_doorbell_range;
> > @@ -143,6 +144,7 @@ typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> > + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> > AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> > AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> > } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> > @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> > AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> > AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
> >
> > + AMDGPU_DOORBELL64_LAST_NON_CP =
> AMDGPU_DOORBELL64_VCE_RING6_7,
> > +
> > AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> > AMDGPU_DOORBELL64_INVALID = 0xFFFF
> > } AMDGPU_DOORBELL64_ASSIGNMENT;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > index 4b5d60ea3e78..fa0433199215 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp = AMDGPU_DOORBELL64_LAST_NON_CP;
> > +
> > /* In unit of dword doorbell */
> > adev->doorbell_index.max_assignment =
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 4;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > index 53716c593b2b..b1052caaff5e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> > +
> > adev->doorbell_index.max_assignment =
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 20;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index e5ebcca7f031..6b8459f852cc 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -103,6 +103,15 @@
> >
> > #define KFD_KERNEL_QUEUE_SIZE 2048
> >
> > +/* 512 = 0x200
> > + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1)
> in the
> > + * same SDMA engine, where i is a even number.
> > + * For 8-bytes doorbells, it ensures that the mirror doorbell range
> (in terms
> > + * of low 12 bit address for each HW engine) on the second doorbell
> page is
> > + * the same as the range of the first doorbell page.*/
> > +#define KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> > +
> > +
> > /*
> > * Kernel module parameter to specify maximum number of supported
> queues per
> > * device
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 80b36e860a0a..c7b67c65d62d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct
> qcm_process_device *qpd,
> > if (!qpd->doorbell_bitmap)
> > return -ENOMEM;
> >
> > - /* Mask out any reserved doorbells */
> > - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> > - if ((dev->shared_resources.reserved_doorbell_mask & i) ==
> > - dev->shared_resources.reserved_doorbell_val) {
> > - set_bit(i, qpd->doorbell_bitmap);
> > - pr_debug("reserved doorbell 0x%03x\n", i);
> > - }
> > + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> > + * Because of the setting in registers like
> SDMA0_DOORBELL_RANGE etc.,
> > + * BIF statically uses the lower 12 bits of doorbell address for
> > + * routing. In order to route the CP queue doorbells to CP engine,
> > + * the doorbells allocated to CP queues have to be outside the
> range
> > + * set for SDMA, VCN, and IH blocks.
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells. */
> > + i = dev->shared_resources.reserved_doorbells_start;
> > + while (i <= dev->shared_resources.reserved_doorbells_end) {
>
> This should be a for-loop to be more obvious:
>
> for (i = dev->shared_resources.reserved_doorbells_start;
> i <= dev->shared_resources.reserved_doorbells_end;
> i++) {
> ...
> }
>
>
> > + set_bit(i, qpd->doorbell_bitmap);
> > + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> > + qpd->doorbell_bitmap);
> > + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> > + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> > +
> > + i++;
> > + }
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > index 83d960110d23..b1bf45419d93 100644
> > --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> > /* Bit n == 1 means Queue n is available for KFD */
> > DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
> >
> > - /* Doorbell assignments (SOC15 and later chips only). Only
> > - * specific doorbells are routed to each SDMA engine. Others
> > - * are routed to IH and VCN. They are not usable by the CP.
> > - *
> > - * Any doorbell number D that satisfies the following condition
> > - * is reserved: (D & reserved_doorbell_mask) ==
> reserved_doorbell_val
> > - *
> > - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> > - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that means
> > - * mask would be set to 0x1e0 and val set to 0x0e0.
> > - */
> > unsigned int sdma_doorbell[2][8];
> > - unsigned int reserved_doorbell_mask;
> > - unsigned int reserved_doorbell_val;
> > +
> > + /* From SOC15 onwards, the doorbell indexes reserved for SDMA, IH,
> > + * and VCN
> > + */
> > + unsigned int reserved_doorbells_start;
> > + unsigned int reserved_doorbells_end;
> >
> > /* Base address of doorbell aperture. */
> > phys_addr_t doorbell_physical_address;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <5aca7b73-2ef6-8a33-4a1c-3f876459de40-5C7GfCeVMHo@public.gmane.org>
@ 2019-02-04 17:15 ` Zeng, Oak
[not found] ` <BL0PR12MB258041F4A571AE1356CA31B8806D0-b4cIHhjg/p/XzH18dTCKOgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Zeng, Oak @ 2019-02-04 17:15 UTC (permalink / raw)
To: Kuehling, Felix, Zhao, Yong, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Correct. MEC_DOORBELL_RANGE should cover both user and kernel CP (KIQ/HIQ/DIQ/amdgpu kernel MEC rings)queues. USERQUEUE_START/END should be renamed to USER_CP_QUEUE_START/END.
Thanks,
Oak
-----Original Message-----
From: Kuehling, Felix
Sent: Monday, February 4, 2019 11:35 AM
To: Zeng, Oak <Oak.Zeng@amd.com>; Zhao, Yong <Yong.Zhao@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
I don't see anything about userqueue start/end in Yong's patch. That said, I think you're mixing up two different things. MEC_DOORBELL_RANGE is not the same as userqueues? We have MEC kernel queues as well that would have to fall in the same range. And we have SDMA user queues that don't belong in the MEC doorbell range. Therefore I think the name USERQUEUE_START/END is misleading.
Regards,
Felix
On 2019-02-02 10:01 a.m., Zeng, Oak wrote:
>
> Hi Yong,
>
> mmCP_MEC_DOORBELL_RANGE_LOWER/UPPER are used at CP to double check a
> doorbell ringing routed to CP. It is a must to program. We have to
> keep userqueue_start/end. Sorry for the confusion.
>
> Regards,
>
> Oak
>
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> *On Behalf Of
> *Zhao, Yong
> *Sent:* Friday, February 1, 2019 7:04 PM
> *To:* Kuehling, Felix <Felix.Kuehling@amd.com>;
> amd-gfx@lists.freedesktop.org; Zeng, Oak <Oak.Zeng@amd.com>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> Oak,
>
> It is a good idea to remove the userqueue_start/end. However, when
> doing so, I came to the following code in gfx_v9_0_kiq_init_register() :
>
> /* enable the doorbell if requested */
>
> if (ring->use_doorbell) {
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
>
> (adev->doorbell_index.kiq * 2) << 2);
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>
> (adev->doorbell_index.userqueue_end * 2) << 2);
>
> }
>
> I remember that you reached the conclusion in one email thread that
> mmCP_MEC_DOORBELL_RANGE_UPPER/LOWER did not have effect at all on
> gfx9. Have we got any firm conclusions from HW engineer in the end? If
> those two registers are still valid, we should enforce the range for
> CP queues.
>
> Regards,
>
> Yong
>
> ----------------------------------------------------------------------
> --
>
> *From:*Kuehling, Felix
> *Sent:* Friday, February 1, 2019 3:03 PM
> *To:* Zhao, Yong; amd-gfx@lists.freedesktop.org
> <mailto:amd-gfx@lists.freedesktop.org>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> > Reserved doorbells for SDMA IH and VCN were not properly masked out
> > when allocating doorbells for CP user queues. This patch fixed that.
> >
> > Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> > Signed-off-by: Yong Zhao <Yong.Zhao@amd.com
> > <mailto:Yong.Zhao@amd.com>>
>
> One mostly cosmetic comment inline. With that fixed, the series is
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com
> <mailto:Felix.Kuehling@amd.com>>
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> > drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25
> >+++++++++++++------
> > .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> > 7 files changed, 56 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index e957e42c539a..13710f34191a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct
> amdgpu_device *adev)
> > gpu_resources.sdma_doorbell[1][i+1] =
> > adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> > }
> > - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> > - * SDMA, IH and VCN. So don't use them for the CP.
> > +
> > + /* Because of the setting in registers like
> > + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> > + * lower 12 bits of doorbell address for routing, in
> > + * order to route the CP queue doorbells to CP engine,
> > + * the doorbells allocated to CP queues have to be
> > + * outside the range set for SDMA, VCN, and IH blocks
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells.
> > */
> > - gpu_resources.reserved_doorbell_mask = 0x1e0;
> > - gpu_resources.reserved_doorbell_val = 0x0e0;
> > + gpu_resources.reserved_doorbells_start =
> > + adev->doorbell_index.sdma_engine[0];
> > + gpu_resources.reserved_doorbells_end =
> > + adev->doorbell_index.last_non_cp;
> >
> > kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > index 1cfec06f81d4..4de431f7f380 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> > uint32_t vce_ring6_7;
> > } uvd_vce;
> > };
> > + uint32_t last_non_cp;
> > uint32_t max_assignment;
> > /* Per engine SDMA doorbell size in dword */
> > uint32_t sdma_doorbell_range; @@ -143,6 +144,7 @@ typedef
> >enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> > + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> > AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> >AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> > } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> > @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> > AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> > AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
> >
> > + AMDGPU_DOORBELL64_LAST_NON_CP =
> AMDGPU_DOORBELL64_VCE_RING6_7,
> > +
> > AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> > AMDGPU_DOORBELL64_INVALID = 0xFFFF
> > } AMDGPU_DOORBELL64_ASSIGNMENT;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > index 4b5d60ea3e78..fa0433199215 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> > +AMDGPU_DOORBELL64_LAST_NON_CP;
> > +
> > /* In unit of dword doorbell */
> > adev->doorbell_index.max_assignment =
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 4; diff --git
> >a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > index 53716c593b2b..b1052caaff5e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> > +
> > adev->doorbell_index.max_assignment =
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 20;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index e5ebcca7f031..6b8459f852cc 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -103,6 +103,15 @@
> >
> > #define KFD_KERNEL_QUEUE_SIZE 2048
> >
> > +/* 512 = 0x200
> > + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1)
> in the
> > + * same SDMA engine, where i is a even number.
> > + * For 8-bytes doorbells, it ensures that the mirror doorbell range
> (in terms
> > + * of low 12 bit address for each HW engine) on the second doorbell
> page is
> > + * the same as the range of the first doorbell page.*/ #define
> > +KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> > +
> > +
> > /*
> > * Kernel module parameter to specify maximum number of supported
> queues per
> > * device
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 80b36e860a0a..c7b67c65d62d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct
> qcm_process_device *qpd,
> > if (!qpd->doorbell_bitmap)
> > return -ENOMEM;
> >
> > - /* Mask out any reserved doorbells */
> > - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> > - if ((dev->shared_resources.reserved_doorbell_mask & i)
> > ==
> > - dev->shared_resources.reserved_doorbell_val) {
> > - set_bit(i, qpd->doorbell_bitmap);
> > - pr_debug("reserved doorbell 0x%03x\n", i);
> > - }
> > + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> > + * Because of the setting in registers like
> SDMA0_DOORBELL_RANGE etc.,
> > + * BIF statically uses the lower 12 bits of doorbell address
> > +for
> > + * routing. In order to route the CP queue doorbells to CP
> > +engine,
> > + * the doorbells allocated to CP queues have to be outside the
> range
> > + * set for SDMA, VCN, and IH blocks.
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells. */
> > + i = dev->shared_resources.reserved_doorbells_start;
> > + while (i <= dev->shared_resources.reserved_doorbells_end) {
>
> This should be a for-loop to be more obvious:
>
> for (i = dev->shared_resources.reserved_doorbells_start;
> i <= dev->shared_resources.reserved_doorbells_end;
> i++) {
> ...
> }
>
>
> > + set_bit(i, qpd->doorbell_bitmap);
> > + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> > + qpd->doorbell_bitmap);
> > + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> > + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> > +
> > + i++;
> > + }
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > index 83d960110d23..b1bf45419d93 100644
> > --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> > /* Bit n == 1 means Queue n is available for KFD */
> > DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
> >
> > - /* Doorbell assignments (SOC15 and later chips only). Only
> > - * specific doorbells are routed to each SDMA engine. Others
> > - * are routed to IH and VCN. They are not usable by the CP.
> > - *
> > - * Any doorbell number D that satisfies the following
> > condition
> > - * is reserved: (D & reserved_doorbell_mask) ==
> reserved_doorbell_val
> > - *
> > - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> > - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that
> >means
> > - * mask would be set to 0x1e0 and val set to 0x0e0.
> > - */
> > unsigned int sdma_doorbell[2][8];
> > - unsigned int reserved_doorbell_mask;
> > - unsigned int reserved_doorbell_val;
> > +
> > + /* From SOC15 onwards, the doorbell indexes reserved for SDMA,
> > +IH,
> > + * and VCN
> > + */
> > + unsigned int reserved_doorbells_start;
> > + unsigned int reserved_doorbells_end;
> >
> > /* Base address of doorbell aperture. */
> > phys_addr_t doorbell_physical_address;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <BL0PR12MB258041F4A571AE1356CA31B8806D0-b4cIHhjg/p/XzH18dTCKOgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-02-04 17:28 ` Zhao, Yong
[not found] ` <SN1PR12MB236591C1FC2D9D965C4DA304F06D0-z7L1TMIYDg7+b/GE5JWDHAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Zhao, Yong @ 2019-02-04 17:28 UTC (permalink / raw)
To: Zeng, Oak, Kuehling, Felix, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 14543 bytes --]
The decision we need to make here is which value we should program mmCP_MEC_DOORBELL_RANGE_UPPER. It is not very consistent right now throughout our code. If we let CP to use all the doorbells left over by SDMA, IH and VCN, then we should probably use 1024 for mmCP_MEC_DOORBELL_RANGE_UPPER. If we continue to use adev->doorbell_index.userqueue_end * 2, as is currently in the code, we should limit the CP doorbells in the range of 0 to adev->doorbell_index.userqueue_end * 2 when allocating CP user queue doorbells.
It seems to me that either way would work.
Regards,
Yong
________________________________
From: Zeng, Oak
Sent: Monday, February 4, 2019 12:15 PM
To: Kuehling, Felix; Zhao, Yong; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: RE: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
Correct. MEC_DOORBELL_RANGE should cover both user and kernel CP (KIQ/HIQ/DIQ/amdgpu kernel MEC rings)queues. USERQUEUE_START/END should be renamed to USER_CP_QUEUE_START/END.
Thanks,
Oak
-----Original Message-----
From: Kuehling, Felix
Sent: Monday, February 4, 2019 11:35 AM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>; Zhao, Yong <Yong.Zhao-5C7GfCeVMHo@public.gmane.org>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
I don't see anything about userqueue start/end in Yong's patch. That said, I think you're mixing up two different things. MEC_DOORBELL_RANGE is not the same as userqueues? We have MEC kernel queues as well that would have to fall in the same range. And we have SDMA user queues that don't belong in the MEC doorbell range. Therefore I think the name USERQUEUE_START/END is misleading.
Regards,
Felix
On 2019-02-02 10:01 a.m., Zeng, Oak wrote:
>
> Hi Yong,
>
> mmCP_MEC_DOORBELL_RANGE_LOWER/UPPER are used at CP to double check a
> doorbell ringing routed to CP. It is a must to program. We have to
> keep userqueue_start/end. Sorry for the confusion.
>
> Regards,
>
> Oak
>
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> *On Behalf Of
> *Zhao, Yong
> *Sent:* Friday, February 1, 2019 7:04 PM
> *To:* Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>;
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> Oak,
>
> It is a good idea to remove the userqueue_start/end. However, when
> doing so, I came to the following code in gfx_v9_0_kiq_init_register() :
>
> /* enable the doorbell if requested */
>
> if (ring->use_doorbell) {
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
>
> (adev->doorbell_index.kiq * 2) << 2);
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>
> (adev->doorbell_index.userqueue_end * 2) << 2);
>
> }
>
> I remember that you reached the conclusion in one email thread that
> mmCP_MEC_DOORBELL_RANGE_UPPER/LOWER did not have effect at all on
> gfx9. Have we got any firm conclusions from HW engineer in the end? If
> those two registers are still valid, we should enforce the range for
> CP queues.
>
> Regards,
>
> Yong
>
> ----------------------------------------------------------------------
> --
>
> *From:*Kuehling, Felix
> *Sent:* Friday, February 1, 2019 3:03 PM
> *To:* Zhao, Yong; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> > Reserved doorbells for SDMA IH and VCN were not properly masked out
> > when allocating doorbells for CP user queues. This patch fixed that.
> >
> > Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> > Signed-off-by: Yong Zhao <Yong.Zhao-5C7GfCeVMHo@public.gmane.org
> > <mailto:Yong.Zhao-5C7GfCeVMHo@public.gmane.org>>
>
> One mostly cosmetic comment inline. With that fixed, the series is
> Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org
> <mailto:Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>>
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> > drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25
> >+++++++++++++------
> > .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> > 7 files changed, 56 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index e957e42c539a..13710f34191a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct
> amdgpu_device *adev)
> > gpu_resources.sdma_doorbell[1][i+1] =
> > adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> > }
> > - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> > - * SDMA, IH and VCN. So don't use them for the CP.
> > +
> > + /* Because of the setting in registers like
> > + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> > + * lower 12 bits of doorbell address for routing, in
> > + * order to route the CP queue doorbells to CP engine,
> > + * the doorbells allocated to CP queues have to be
> > + * outside the range set for SDMA, VCN, and IH blocks
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells.
> > */
> > - gpu_resources.reserved_doorbell_mask = 0x1e0;
> > - gpu_resources.reserved_doorbell_val = 0x0e0;
> > + gpu_resources.reserved_doorbells_start =
> > + adev->doorbell_index.sdma_engine[0];
> > + gpu_resources.reserved_doorbells_end =
> > + adev->doorbell_index.last_non_cp;
> >
> > kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > index 1cfec06f81d4..4de431f7f380 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> > uint32_t vce_ring6_7;
> > } uvd_vce;
> > };
> > + uint32_t last_non_cp;
> > uint32_t max_assignment;
> > /* Per engine SDMA doorbell size in dword */
> > uint32_t sdma_doorbell_range; @@ -143,6 +144,7 @@ typedef
> >enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> > + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> > AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> >AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> > } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> > @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> > AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> > AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
> >
> > + AMDGPU_DOORBELL64_LAST_NON_CP =
> AMDGPU_DOORBELL64_VCE_RING6_7,
> > +
> > AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> > AMDGPU_DOORBELL64_INVALID = 0xFFFF
> > } AMDGPU_DOORBELL64_ASSIGNMENT;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > index 4b5d60ea3e78..fa0433199215 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> > +AMDGPU_DOORBELL64_LAST_NON_CP;
> > +
> > /* In unit of dword doorbell */
> > adev->doorbell_index.max_assignment =
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 4; diff --git
> >a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > index 53716c593b2b..b1052caaff5e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> > +
> > adev->doorbell_index.max_assignment =
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 20;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index e5ebcca7f031..6b8459f852cc 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -103,6 +103,15 @@
> >
> > #define KFD_KERNEL_QUEUE_SIZE 2048
> >
> > +/* 512 = 0x200
> > + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1)
> in the
> > + * same SDMA engine, where i is a even number.
> > + * For 8-bytes doorbells, it ensures that the mirror doorbell range
> (in terms
> > + * of low 12 bit address for each HW engine) on the second doorbell
> page is
> > + * the same as the range of the first doorbell page.*/ #define
> > +KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> > +
> > +
> > /*
> > * Kernel module parameter to specify maximum number of supported
> queues per
> > * device
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 80b36e860a0a..c7b67c65d62d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct
> qcm_process_device *qpd,
> > if (!qpd->doorbell_bitmap)
> > return -ENOMEM;
> >
> > - /* Mask out any reserved doorbells */
> > - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> > - if ((dev->shared_resources.reserved_doorbell_mask & i)
> > ==
> > - dev->shared_resources.reserved_doorbell_val) {
> > - set_bit(i, qpd->doorbell_bitmap);
> > - pr_debug("reserved doorbell 0x%03x\n", i);
> > - }
> > + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> > + * Because of the setting in registers like
> SDMA0_DOORBELL_RANGE etc.,
> > + * BIF statically uses the lower 12 bits of doorbell address
> > +for
> > + * routing. In order to route the CP queue doorbells to CP
> > +engine,
> > + * the doorbells allocated to CP queues have to be outside the
> range
> > + * set for SDMA, VCN, and IH blocks.
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells. */
> > + i = dev->shared_resources.reserved_doorbells_start;
> > + while (i <= dev->shared_resources.reserved_doorbells_end) {
>
> This should be a for-loop to be more obvious:
>
> for (i = dev->shared_resources.reserved_doorbells_start;
> i <= dev->shared_resources.reserved_doorbells_end;
> i++) {
> ...
> }
>
>
> > + set_bit(i, qpd->doorbell_bitmap);
> > + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> > + qpd->doorbell_bitmap);
> > + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> > + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> > +
> > + i++;
> > + }
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > index 83d960110d23..b1bf45419d93 100644
> > --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> > /* Bit n == 1 means Queue n is available for KFD */
> > DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
> >
> > - /* Doorbell assignments (SOC15 and later chips only). Only
> > - * specific doorbells are routed to each SDMA engine. Others
> > - * are routed to IH and VCN. They are not usable by the CP.
> > - *
> > - * Any doorbell number D that satisfies the following
> > condition
> > - * is reserved: (D & reserved_doorbell_mask) ==
> reserved_doorbell_val
> > - *
> > - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> > - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that
> >means
> > - * mask would be set to 0x1e0 and val set to 0x0e0.
> > - */
> > unsigned int sdma_doorbell[2][8];
> > - unsigned int reserved_doorbell_mask;
> > - unsigned int reserved_doorbell_val;
> > +
> > + /* From SOC15 onwards, the doorbell indexes reserved for SDMA,
> > +IH,
> > + * and VCN
> > + */
> > + unsigned int reserved_doorbells_start;
> > + unsigned int reserved_doorbells_end;
> >
> > /* Base address of doorbell aperture. */
> > phys_addr_t doorbell_physical_address;
>
[-- Attachment #1.2: Type: text/html, Size: 25255 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
[not found] ` <SN1PR12MB236591C1FC2D9D965C4DA304F06D0-z7L1TMIYDg7+b/GE5JWDHAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-02-04 17:32 ` Zeng, Oak
0 siblings, 0 replies; 12+ messages in thread
From: Zeng, Oak @ 2019-02-04 17:32 UTC (permalink / raw)
To: Zhao, Yong, Kuehling, Felix, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 15642 bytes --]
>then we should probably use 1024 for mmCP_MEC_DOORBELL_RANGE_UPPER.
This is also what I thought.
Thanks,
Oak
From: Zhao, Yong
Sent: Monday, February 4, 2019 12:28 PM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>; Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
The decision we need to make here is which value we should program mmCP_MEC_DOORBELL_RANGE_UPPER. It is not very consistent right now throughout our code. If we let CP to use all the doorbells left over by SDMA, IH and VCN, then we should probably use 1024 for mmCP_MEC_DOORBELL_RANGE_UPPER. If we continue to use adev->doorbell_index.userqueue_end * 2, as is currently in the code, we should limit the CP doorbells in the range of 0 to adev->doorbell_index.userqueue_end * 2 when allocating CP user queue doorbells.
It seems to me that either way would work.
Regards,
Yong
________________________________
From: Zeng, Oak
Sent: Monday, February 4, 2019 12:15 PM
To: Kuehling, Felix; Zhao, Yong; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: RE: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
Correct. MEC_DOORBELL_RANGE should cover both user and kernel CP (KIQ/HIQ/DIQ/amdgpu kernel MEC rings)queues. USERQUEUE_START/END should be renamed to USER_CP_QUEUE_START/END.
Thanks,
Oak
-----Original Message-----
From: Kuehling, Felix
Sent: Monday, February 4, 2019 11:35 AM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>; Zhao, Yong <Yong.Zhao-5C7GfCeVMHo@public.gmane.org<mailto:Yong.Zhao-5C7GfCeVMHo@public.gmane.org>>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15
I don't see anything about userqueue start/end in Yong's patch. That said, I think you're mixing up two different things. MEC_DOORBELL_RANGE is not the same as userqueues? We have MEC kernel queues as well that would have to fall in the same range. And we have SDMA user queues that don't belong in the MEC doorbell range. Therefore I think the name USERQUEUE_START/END is misleading.
Regards,
Felix
On 2019-02-02 10:01 a.m., Zeng, Oak wrote:
>
> Hi Yong,
>
> mmCP_MEC_DOORBELL_RANGE_LOWER/UPPER are used at CP to double check a
> doorbell ringing routed to CP. It is a must to program. We have to
> keep userqueue_start/end. Sorry for the confusion.
>
> Regards,
>
> Oak
>
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>> *On Behalf Of
> *Zhao, Yong
> *Sent:* Friday, February 1, 2019 7:04 PM
> *To:* Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org<mailto:Felix.Kuehling@amd.com>>;
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>; Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> Oak,
>
> It is a good idea to remove the userqueue_start/end. However, when
> doing so, I came to the following code in gfx_v9_0_kiq_init_register() :
>
> /* enable the doorbell if requested */
>
> if (ring->use_doorbell) {
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
>
> (adev->doorbell_index.kiq * 2) << 2);
>
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>
> (adev->doorbell_index.userqueue_end * 2) << 2);
>
> }
>
> I remember that you reached the conclusion in one email thread that
> mmCP_MEC_DOORBELL_RANGE_UPPER/LOWER did not have effect at all on
> gfx9. Have we got any firm conclusions from HW engineer in the end? If
> those two registers are still valid, we should enforce the range for
> CP queues.
>
> Regards,
>
> Yong
>
> ----------------------------------------------------------------------
> --
>
> *From:*Kuehling, Felix
> *Sent:* Friday, February 1, 2019 3:03 PM
> *To:* Zhao, Yong; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32mptlylMvRsHA@public.gmane.orgdesktop.org>
> <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user
> queue doorbells mask on SOC15
>
> On 2019-01-31 5:27 p.m., Zhao, Yong wrote:
> > Reserved doorbells for SDMA IH and VCN were not properly masked out
> > when allocating doorbells for CP user queues. This patch fixed that.
> >
> > Change-Id: I670adfc3fd7725d2ed0bd9665cb7f69f8b9023c2
> > Signed-off-by: Yong Zhao <Yong.Zhao-5C7GfCeVMHo@public.gmane.org
<mailto:Yong.Zhao-5C7GfCeVMHo@public.gmane.org%20%0b>> > <mailto:Yong.Zhao-5C7GfCeVMHo@public.gmane.org>>
>
> One mostly cosmetic comment inline. With that fixed, the series is
> Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org
<mailto:Felix.Kuehling-5C7GfCeVMHo@public.gmane.org%20%0b>> <mailto:Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>>
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 17 ++++++++++---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 4 +++
> > drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 3 +++
> > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 9 +++++++
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 25
> >+++++++++++++------
> > .../gpu/drm/amd/include/kgd_kfd_interface.h | 19 +++++---------
> > 7 files changed, 56 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index e957e42c539a..13710f34191a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -196,11 +196,20 @@ void amdgpu_amdkfd_device_init(struct
> amdgpu_device *adev)
> > gpu_resources.sdma_doorbell[1][i+1] =
> > adev->doorbell_index.sdma_engine[1] + 0x200 + (i >> 1);
> > }
> > - /* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
> > - * SDMA, IH and VCN. So don't use them for the CP.
> > +
> > + /* Because of the setting in registers like
> > + * SDMA0_DOORBELL_RANGE etc., BIF statically uses the
> > + * lower 12 bits of doorbell address for routing, in
> > + * order to route the CP queue doorbells to CP engine,
> > + * the doorbells allocated to CP queues have to be
> > + * outside the range set for SDMA, VCN, and IH blocks
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells.
> > */
> > - gpu_resources.reserved_doorbell_mask = 0x1e0;
> > - gpu_resources.reserved_doorbell_val = 0x0e0;
> > + gpu_resources.reserved_doorbells_start =
> > + adev->doorbell_index.sdma_engine[0];
> > + gpu_resources.reserved_doorbells_end =
> > + adev->doorbell_index.last_non_cp;
> >
> > kgd2kfd_device_init(adev->kfd.dev, &gpu_resources);
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > index 1cfec06f81d4..4de431f7f380 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> > @@ -71,6 +71,7 @@ struct amdgpu_doorbell_index {
> > uint32_t vce_ring6_7;
> > } uvd_vce;
> > };
> > + uint32_t last_non_cp;
> > uint32_t max_assignment;
> > /* Per engine SDMA doorbell size in dword */
> > uint32_t sdma_doorbell_range; @@ -143,6 +144,7 @@ typedef
> >enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
> > AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
> > + AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7,
> > AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT = 0x18F,
> >AMDGPU_VEGA20_DOORBELL_INVALID = 0xFFFF
> > } AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
> > @@ -222,6 +224,8 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
> > AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE,
> > AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF,
> >
> > + AMDGPU_DOORBELL64_LAST_NON_CP =
> AMDGPU_DOORBELL64_VCE_RING6_7,
> > +
> > AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF,
> > AMDGPU_DOORBELL64_INVALID = 0xFFFF
> > } AMDGPU_DOORBELL64_ASSIGNMENT;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > index 4b5d60ea3e78..fa0433199215 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
> > @@ -81,6 +81,9 @@ void vega10_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> > +AMDGPU_DOORBELL64_LAST_NON_CP;
> > +
> > /* In unit of dword doorbell */
> > adev->doorbell_index.max_assignment =
> AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 4; diff --git
> >a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > index 53716c593b2b..b1052caaff5e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> > @@ -85,6 +85,9 @@ void vega20_doorbell_index_init(struct
> amdgpu_device *adev)
> > adev->doorbell_index.uvd_vce.vce_ring2_3 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3;
> > adev->doorbell_index.uvd_vce.vce_ring4_5 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
> > adev->doorbell_index.uvd_vce.vce_ring6_7 =
> AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
> > +
> > + adev->doorbell_index.last_non_cp =
> AMDGPU_VEGA20_DOORBELL64_LAST_NON_CP;
> > +
> > adev->doorbell_index.max_assignment =
> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
> > adev->doorbell_index.sdma_doorbell_range = 20;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index e5ebcca7f031..6b8459f852cc 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -103,6 +103,15 @@
> >
> > #define KFD_KERNEL_QUEUE_SIZE 2048
> >
> > +/* 512 = 0x200
> > + * On SOC15, the doorbell index distance for SDMA RLC i and (i + 1)
> in the
> > + * same SDMA engine, where i is a even number.
> > + * For 8-bytes doorbells, it ensures that the mirror doorbell range
> (in terms
> > + * of low 12 bit address for each HW engine) on the second doorbell
> page is
> > + * the same as the range of the first doorbell page.*/ #define
> > +KFD_QUEUE_DOORBELL_MIRROR_OFFSET 512
> > +
> > +
> > /*
> > * Kernel module parameter to specify maximum number of supported
> queues per
> > * device
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 80b36e860a0a..c7b67c65d62d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -607,13 +607,24 @@ static int init_doorbell_bitmap(struct
> qcm_process_device *qpd,
> > if (!qpd->doorbell_bitmap)
> > return -ENOMEM;
> >
> > - /* Mask out any reserved doorbells */
> > - for (i = 0; i < KFD_MAX_NUM_OF_QUEUES_PER_PROCESS; i++)
> > - if ((dev->shared_resources.reserved_doorbell_mask & i)
> > ==
> > - dev->shared_resources.reserved_doorbell_val) {
> > - set_bit(i, qpd->doorbell_bitmap);
> > - pr_debug("reserved doorbell 0x%03x\n", i);
> > - }
> > + /* Mask out all reserved doorbells for SDMA, IH, and VCN on SOC15.
> > + * Because of the setting in registers like
> SDMA0_DOORBELL_RANGE etc.,
> > + * BIF statically uses the lower 12 bits of doorbell address
> > +for
> > + * routing. In order to route the CP queue doorbells to CP
> > +engine,
> > + * the doorbells allocated to CP queues have to be outside the
> range
> > + * set for SDMA, VCN, and IH blocks.
> > + * Prior to SOC15, all queues use queue ID to
> > + * determine doorbells. */
> > + i = dev->shared_resources.reserved_doorbells_start;
> > + while (i <= dev->shared_resources.reserved_doorbells_end) {
>
> This should be a for-loop to be more obvious:
>
> for (i = dev->shared_resources.reserved_doorbells_start;
> i <= dev->shared_resources.reserved_doorbells_end;
> i++) {
> ...
> }
>
>
> > + set_bit(i, qpd->doorbell_bitmap);
> > + set_bit(i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET,
> > + qpd->doorbell_bitmap);
> > + pr_debug("reserved doorbell 0x%03x and 0x%03x\n", i,
> > + i + KFD_QUEUE_DOORBELL_MIRROR_OFFSET);
> > +
> > + i++;
> > + }
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > index 83d960110d23..b1bf45419d93 100644
> > --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> > @@ -137,20 +137,13 @@ struct kgd2kfd_shared_resources {
> > /* Bit n == 1 means Queue n is available for KFD */
> > DECLARE_BITMAP(queue_bitmap, KGD_MAX_QUEUES);
> >
> > - /* Doorbell assignments (SOC15 and later chips only). Only
> > - * specific doorbells are routed to each SDMA engine. Others
> > - * are routed to IH and VCN. They are not usable by the CP.
> > - *
> > - * Any doorbell number D that satisfies the following
> > condition
> > - * is reserved: (D & reserved_doorbell_mask) ==
> reserved_doorbell_val
> > - *
> > - * KFD currently uses 1024 (= 0x3ff) doorbells per process. If
> > - * doorbells 0x0e0-0x0ff and 0x2e0-0x2ff are reserved, that
> >means
> > - * mask would be set to 0x1e0 and val set to 0x0e0.
> > - */
> > unsigned int sdma_doorbell[2][8];
> > - unsigned int reserved_doorbell_mask;
> > - unsigned int reserved_doorbell_val;
> > +
> > + /* From SOC15 onwards, the doorbell indexes reserved for SDMA,
> > +IH,
> > + * and VCN
> > + */
> > + unsigned int reserved_doorbells_start;
> > + unsigned int reserved_doorbells_end;
> >
> > /* Base address of doorbell aperture. */
> > phys_addr_t doorbell_physical_address;
>
[-- Attachment #1.2: Type: text/html, Size: 28429 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-04 17:32 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 22:27 [PATCH 1/3] drm/amdkfd: Move a constant definition around Zhao, Yong
[not found] ` <20190131222703.3591-1-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-01-31 22:27 ` [PATCH 2/3] drm/amdkfd: Fix bugs regarding CP user queue doorbells mask on SOC15 Zhao, Yong
[not found] ` <20190131222703.3591-2-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-02-01 16:58 ` Zeng, Oak
2019-02-01 20:03 ` Kuehling, Felix
[not found] ` <e01f29c1-74e8-4dd6-81fc-b25c0432798f-5C7GfCeVMHo@public.gmane.org>
2019-02-02 0:04 ` Zhao, Yong
[not found] ` <BL0PR12MB250046784085018A70E34EC0F0920-b4cIHhjg/p89/wkoUb0eEAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-02-02 15:01 ` Zeng, Oak
[not found] ` <DM5PR12MB25826F20FB8EF1E7E4B6348480930-2J9CzHegvk8nywYgqlzn1wdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-02-04 16:34 ` Kuehling, Felix
[not found] ` <5aca7b73-2ef6-8a33-4a1c-3f876459de40-5C7GfCeVMHo@public.gmane.org>
2019-02-04 17:15 ` Zeng, Oak
[not found] ` <BL0PR12MB258041F4A571AE1356CA31B8806D0-b4cIHhjg/p/XzH18dTCKOgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-02-04 17:28 ` Zhao, Yong
[not found] ` <SN1PR12MB236591C1FC2D9D965C4DA304F06D0-z7L1TMIYDg7+b/GE5JWDHAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-02-04 17:32 ` Zeng, Oak
2019-01-31 22:27 ` [PATCH 3/3] drm/amdkfd: Optimize out sdma doorbell array in kgd2kfd_shared_resources Zhao, Yong
[not found] ` <20190131222703.3591-3-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-02-01 17:09 ` Zeng, Oak
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.