* [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-01 20:10 ` Zhao, Yong
0 siblings, 0 replies; 14+ messages in thread
From: Zhao, Yong @ 2019-11-01 20:10 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
dorbell_off in the queue properties is mainly used for the doorbell dw
offset in pci bar. We should not set it to the doorbell byte offset in
process doorbell pages. This makes the code much easier to read.
Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
.../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index d9e36dbf13d5..b91993753b82 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
unsigned int queue_id;
struct kfd_process_device *pdd;
struct queue_properties q_properties;
+ uint32_t doorbell_offset_in_process = 0;
memset(&q_properties, 0, sizeof(struct queue_properties));
@@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
p->pasid,
dev->id);
- err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
+ err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
+ &doorbell_offset_in_process);
if (err != 0)
goto err_create_queue;
@@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
args->doorbell_offset <<= PAGE_SHIFT;
if (KFD_IS_SOC15(dev->device_info->asic_family))
- /* On SOC15 ASICs, doorbell allocation must be
- * per-device, and independent from the per-process
- * queue_id. Return the doorbell offset within the
- * doorbell aperture to user mode.
+ /* On SOC15 ASICs, include the doorbell offset within the
+ * process doorbell frame, which could be 1 page or 2 pages.
*/
- args->doorbell_offset |= q_properties.doorbell_off;
+ args->doorbell_offset |= doorbell_offset_in_process;
mutex_unlock(&p->mutex);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
index d59f2cd056c6..1d33c4f25263 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
@@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
properties.type = KFD_QUEUE_TYPE_DIQ;
status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
- &properties, &qid);
+ &properties, &qid, NULL);
if (status) {
pr_err("Failed to create DIQ\n");
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 7c561c98f2e2..66bae8f2dad1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev,
struct file *f,
struct queue_properties *properties,
- unsigned int *qid);
+ unsigned int *qid,
+ uint32_t *p_doorbell_offset_in_process);
int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
struct queue_properties *p);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 8509814a6ff0..48185d2957e9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev,
struct file *f,
struct queue_properties *properties,
- unsigned int *qid)
+ unsigned int *qid,
+ uint32_t *p_doorbell_offset_in_process)
{
int retval;
struct kfd_process_device *pdd;
@@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
/* Return the doorbell offset within the doorbell page
* to the caller so it can be passed up to user mode
* (in bytes).
+ * There are always 1024 doorbells per process, so in case
+ * of 8-byte doorbells, there are two doorbell pages per
+ * process.
*/
- properties->doorbell_off =
+ *p_doorbell_offset_in_process =
(q->properties.doorbell_off * sizeof(uint32_t)) &
(kfd_doorbell_process_slice(dev) - 1);
--
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] 14+ messages in thread
* [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords
@ 2019-11-01 20:10 ` Zhao, Yong
0 siblings, 0 replies; 14+ messages in thread
From: Zhao, Yong @ 2019-11-01 20:10 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Zhao, Yong
Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 14 +++++++-------
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++----
3 files changed, 12 insertions(+), 12 deletions(-)
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 984c2f2b24b6..4503fb26fe5b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
}
q->properties.doorbell_off =
- kfd_doorbell_id_to_offset(dev, q->process,
+ kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
q->doorbell_id);
return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index ebe79bf00145..f904355c44a1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
doorbell_start_offset;
- kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
+ kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
kfd_doorbell_process_slice(kfd));
@@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
pr_debug("doorbell base == 0x%08lX\n",
(uintptr_t)kfd->doorbell_base);
- pr_debug("doorbell_id_offset == 0x%08lX\n",
- kfd->doorbell_id_offset);
+ pr_debug("doorbell_base_dw_offset == 0x%08lX\n",
+ kfd->doorbell_base_dw_offset);
pr_debug("doorbell_process_limit == 0x%08lX\n",
doorbell_process_limit);
@@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
* Calculating the kernel doorbell offset using the first
* doorbell page.
*/
- *doorbell_off = kfd->doorbell_id_offset + inx;
+ *doorbell_off = kfd->doorbell_base_dw_offset + inx;
pr_debug("Get kernel queue doorbell\n"
" doorbell offset == 0x%08X\n"
@@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
}
}
-unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
+unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
struct kfd_process *process,
unsigned int doorbell_id)
{
/*
- * doorbell_id_offset accounts for doorbells taken by KGD.
+ * doorbell_base_dw_offset accounts for doorbells taken by KGD.
* index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
* the process's doorbells. The offset returned is in dword
* units regardless of the ASIC-dependent doorbell size.
*/
- return kfd->doorbell_id_offset +
+ return kfd->doorbell_base_dw_offset +
process->doorbell_index
* kfd_doorbell_process_slice(kfd) / sizeof(u32) +
doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 62db4d20ed32..7c561c98f2e2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -238,9 +238,9 @@ struct kfd_dev {
* KFD. It is aligned for mapping
* into user mode
*/
- size_t doorbell_id_offset; /* Doorbell offset (from KFD doorbell
- * to HW doorbell, GFX reserved some
- * at the start)
+ size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
+ * doorbell to PCI doorbell bar,
+ * GFX reserved some at the start)
*/
u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
* page used by kernel queue
@@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 __iomem *db_addr);
u32 read_kernel_doorbell(u32 __iomem *db);
void write_kernel_doorbell(void __iomem *db, u32 value);
void write_kernel_doorbell64(void __iomem *db, u64 value);
-unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
+unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
struct kfd_process *process,
unsigned int doorbell_id);
phys_addr_t kfd_get_process_doorbells(struct kfd_dev *dev,
--
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] 14+ messages in thread
* [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-01 20:10 ` Zhao, Yong
0 siblings, 0 replies; 14+ messages in thread
From: Zhao, Yong @ 2019-11-01 20:10 UTC (permalink / raw)
To: amd-gfx; +Cc: Zhao, Yong
dorbell_off in the queue properties is mainly used for the doorbell dw
offset in pci bar. We should not set it to the doorbell byte offset in
process doorbell pages. This makes the code much easier to read.
Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
.../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index d9e36dbf13d5..b91993753b82 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
unsigned int queue_id;
struct kfd_process_device *pdd;
struct queue_properties q_properties;
+ uint32_t doorbell_offset_in_process = 0;
memset(&q_properties, 0, sizeof(struct queue_properties));
@@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
p->pasid,
dev->id);
- err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
+ err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
+ &doorbell_offset_in_process);
if (err != 0)
goto err_create_queue;
@@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
args->doorbell_offset <<= PAGE_SHIFT;
if (KFD_IS_SOC15(dev->device_info->asic_family))
- /* On SOC15 ASICs, doorbell allocation must be
- * per-device, and independent from the per-process
- * queue_id. Return the doorbell offset within the
- * doorbell aperture to user mode.
+ /* On SOC15 ASICs, include the doorbell offset within the
+ * process doorbell frame, which could be 1 page or 2 pages.
*/
- args->doorbell_offset |= q_properties.doorbell_off;
+ args->doorbell_offset |= doorbell_offset_in_process;
mutex_unlock(&p->mutex);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
index d59f2cd056c6..1d33c4f25263 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
@@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
properties.type = KFD_QUEUE_TYPE_DIQ;
status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
- &properties, &qid);
+ &properties, &qid, NULL);
if (status) {
pr_err("Failed to create DIQ\n");
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 7c561c98f2e2..66bae8f2dad1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev,
struct file *f,
struct queue_properties *properties,
- unsigned int *qid);
+ unsigned int *qid,
+ uint32_t *p_doorbell_offset_in_process);
int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
struct queue_properties *p);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 8509814a6ff0..48185d2957e9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev,
struct file *f,
struct queue_properties *properties,
- unsigned int *qid)
+ unsigned int *qid,
+ uint32_t *p_doorbell_offset_in_process)
{
int retval;
struct kfd_process_device *pdd;
@@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
/* Return the doorbell offset within the doorbell page
* to the caller so it can be passed up to user mode
* (in bytes).
+ * There are always 1024 doorbells per process, so in case
+ * of 8-byte doorbells, there are two doorbell pages per
+ * process.
*/
- properties->doorbell_off =
+ *p_doorbell_offset_in_process =
(q->properties.doorbell_off * sizeof(uint32_t)) &
(kfd_doorbell_process_slice(dev) - 1);
--
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] 14+ messages in thread
* [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords
@ 2019-11-01 20:10 ` Zhao, Yong
0 siblings, 0 replies; 14+ messages in thread
From: Zhao, Yong @ 2019-11-01 20:10 UTC (permalink / raw)
To: amd-gfx; +Cc: Zhao, Yong
Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 14 +++++++-------
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++----
3 files changed, 12 insertions(+), 12 deletions(-)
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 984c2f2b24b6..4503fb26fe5b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
}
q->properties.doorbell_off =
- kfd_doorbell_id_to_offset(dev, q->process,
+ kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
q->doorbell_id);
return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index ebe79bf00145..f904355c44a1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
doorbell_start_offset;
- kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
+ kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
kfd_doorbell_process_slice(kfd));
@@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
pr_debug("doorbell base == 0x%08lX\n",
(uintptr_t)kfd->doorbell_base);
- pr_debug("doorbell_id_offset == 0x%08lX\n",
- kfd->doorbell_id_offset);
+ pr_debug("doorbell_base_dw_offset == 0x%08lX\n",
+ kfd->doorbell_base_dw_offset);
pr_debug("doorbell_process_limit == 0x%08lX\n",
doorbell_process_limit);
@@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
* Calculating the kernel doorbell offset using the first
* doorbell page.
*/
- *doorbell_off = kfd->doorbell_id_offset + inx;
+ *doorbell_off = kfd->doorbell_base_dw_offset + inx;
pr_debug("Get kernel queue doorbell\n"
" doorbell offset == 0x%08X\n"
@@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
}
}
-unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
+unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
struct kfd_process *process,
unsigned int doorbell_id)
{
/*
- * doorbell_id_offset accounts for doorbells taken by KGD.
+ * doorbell_base_dw_offset accounts for doorbells taken by KGD.
* index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
* the process's doorbells. The offset returned is in dword
* units regardless of the ASIC-dependent doorbell size.
*/
- return kfd->doorbell_id_offset +
+ return kfd->doorbell_base_dw_offset +
process->doorbell_index
* kfd_doorbell_process_slice(kfd) / sizeof(u32) +
doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 62db4d20ed32..7c561c98f2e2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -238,9 +238,9 @@ struct kfd_dev {
* KFD. It is aligned for mapping
* into user mode
*/
- size_t doorbell_id_offset; /* Doorbell offset (from KFD doorbell
- * to HW doorbell, GFX reserved some
- * at the start)
+ size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
+ * doorbell to PCI doorbell bar,
+ * GFX reserved some at the start)
*/
u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
* page used by kernel queue
@@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 __iomem *db_addr);
u32 read_kernel_doorbell(u32 __iomem *db);
void write_kernel_doorbell(void __iomem *db, u32 value);
void write_kernel_doorbell64(void __iomem *db, u64 value);
-unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
+unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
struct kfd_process *process,
unsigned int doorbell_id);
phys_addr_t kfd_get_process_doorbells(struct kfd_dev *dev,
--
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] 14+ messages in thread
* Re: [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords
@ 2019-11-11 18:04 ` Yong Zhao
0 siblings, 0 replies; 14+ messages in thread
From: Yong Zhao @ 2019-11-11 18:04 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx list
ping
On 2019-11-01 4:10 p.m., Zhao, Yong wrote:
> Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 14 +++++++-------
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> 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 984c2f2b24b6..4503fb26fe5b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
> }
>
> q->properties.doorbell_off =
> - kfd_doorbell_id_to_offset(dev, q->process,
> + kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
> q->doorbell_id);
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> index ebe79bf00145..f904355c44a1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> @@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
> doorbell_start_offset;
>
> - kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
> + kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
>
> kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
> kfd_doorbell_process_slice(kfd));
> @@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> pr_debug("doorbell base == 0x%08lX\n",
> (uintptr_t)kfd->doorbell_base);
>
> - pr_debug("doorbell_id_offset == 0x%08lX\n",
> - kfd->doorbell_id_offset);
> + pr_debug("doorbell_base_dw_offset == 0x%08lX\n",
> + kfd->doorbell_base_dw_offset);
>
> pr_debug("doorbell_process_limit == 0x%08lX\n",
> doorbell_process_limit);
> @@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
> * Calculating the kernel doorbell offset using the first
> * doorbell page.
> */
> - *doorbell_off = kfd->doorbell_id_offset + inx;
> + *doorbell_off = kfd->doorbell_base_dw_offset + inx;
>
> pr_debug("Get kernel queue doorbell\n"
> " doorbell offset == 0x%08X\n"
> @@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
> }
> }
>
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id)
> {
> /*
> - * doorbell_id_offset accounts for doorbells taken by KGD.
> + * doorbell_base_dw_offset accounts for doorbells taken by KGD.
> * index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
> * the process's doorbells. The offset returned is in dword
> * units regardless of the ASIC-dependent doorbell size.
> */
> - return kfd->doorbell_id_offset +
> + return kfd->doorbell_base_dw_offset +
> process->doorbell_index
> * kfd_doorbell_process_slice(kfd) / sizeof(u32) +
> doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 62db4d20ed32..7c561c98f2e2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -238,9 +238,9 @@ struct kfd_dev {
> * KFD. It is aligned for mapping
> * into user mode
> */
> - size_t doorbell_id_offset; /* Doorbell offset (from KFD doorbell
> - * to HW doorbell, GFX reserved some
> - * at the start)
> + size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
> + * doorbell to PCI doorbell bar,
> + * GFX reserved some at the start)
> */
> u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
> * page used by kernel queue
> @@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 __iomem *db_addr);
> u32 read_kernel_doorbell(u32 __iomem *db);
> void write_kernel_doorbell(void __iomem *db, u32 value);
> void write_kernel_doorbell64(void __iomem *db, u64 value);
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id);
> phys_addr_t kfd_get_process_doorbells(struct kfd_dev *dev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords
@ 2019-11-11 18:04 ` Yong Zhao
0 siblings, 0 replies; 14+ messages in thread
From: Yong Zhao @ 2019-11-11 18:04 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx list
ping
On 2019-11-01 4:10 p.m., Zhao, Yong wrote:
> Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 14 +++++++-------
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> 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 984c2f2b24b6..4503fb26fe5b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
> }
>
> q->properties.doorbell_off =
> - kfd_doorbell_id_to_offset(dev, q->process,
> + kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
> q->doorbell_id);
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> index ebe79bf00145..f904355c44a1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> @@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
> doorbell_start_offset;
>
> - kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
> + kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
>
> kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
> kfd_doorbell_process_slice(kfd));
> @@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> pr_debug("doorbell base == 0x%08lX\n",
> (uintptr_t)kfd->doorbell_base);
>
> - pr_debug("doorbell_id_offset == 0x%08lX\n",
> - kfd->doorbell_id_offset);
> + pr_debug("doorbell_base_dw_offset == 0x%08lX\n",
> + kfd->doorbell_base_dw_offset);
>
> pr_debug("doorbell_process_limit == 0x%08lX\n",
> doorbell_process_limit);
> @@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
> * Calculating the kernel doorbell offset using the first
> * doorbell page.
> */
> - *doorbell_off = kfd->doorbell_id_offset + inx;
> + *doorbell_off = kfd->doorbell_base_dw_offset + inx;
>
> pr_debug("Get kernel queue doorbell\n"
> " doorbell offset == 0x%08X\n"
> @@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
> }
> }
>
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id)
> {
> /*
> - * doorbell_id_offset accounts for doorbells taken by KGD.
> + * doorbell_base_dw_offset accounts for doorbells taken by KGD.
> * index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
> * the process's doorbells. The offset returned is in dword
> * units regardless of the ASIC-dependent doorbell size.
> */
> - return kfd->doorbell_id_offset +
> + return kfd->doorbell_base_dw_offset +
> process->doorbell_index
> * kfd_doorbell_process_slice(kfd) / sizeof(u32) +
> doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 62db4d20ed32..7c561c98f2e2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -238,9 +238,9 @@ struct kfd_dev {
> * KFD. It is aligned for mapping
> * into user mode
> */
> - size_t doorbell_id_offset; /* Doorbell offset (from KFD doorbell
> - * to HW doorbell, GFX reserved some
> - * at the start)
> + size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
> + * doorbell to PCI doorbell bar,
> + * GFX reserved some at the start)
> */
> u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
> * page used by kernel queue
> @@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 __iomem *db_addr);
> u32 read_kernel_doorbell(u32 __iomem *db);
> void write_kernel_doorbell(void __iomem *db, u32 value);
> void write_kernel_doorbell64(void __iomem *db, u64 value);
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id);
> phys_addr_t kfd_get_process_doorbells(struct kfd_dev *dev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords
@ 2019-11-11 20:29 ` Felix Kuehling
0 siblings, 0 replies; 14+ messages in thread
From: Felix Kuehling @ 2019-11-11 20:29 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 2019-11-01 16:10, Zhao, Yong wrote:
> Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
I agree with the name changes. One suggestion for a comment inline. With
that fixed, this patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 14 +++++++-------
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> 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 984c2f2b24b6..4503fb26fe5b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
> }
>
> q->properties.doorbell_off =
> - kfd_doorbell_id_to_offset(dev, q->process,
> + kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
> q->doorbell_id);
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> index ebe79bf00145..f904355c44a1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> @@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
> doorbell_start_offset;
>
> - kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
> + kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
>
> kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
> kfd_doorbell_process_slice(kfd));
> @@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> pr_debug("doorbell base == 0x%08lX\n",
> (uintptr_t)kfd->doorbell_base);
>
> - pr_debug("doorbell_id_offset == 0x%08lX\n",
> - kfd->doorbell_id_offset);
> + pr_debug("doorbell_base_dw_offset == 0x%08lX\n",
> + kfd->doorbell_base_dw_offset);
>
> pr_debug("doorbell_process_limit == 0x%08lX\n",
> doorbell_process_limit);
> @@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
> * Calculating the kernel doorbell offset using the first
> * doorbell page.
> */
> - *doorbell_off = kfd->doorbell_id_offset + inx;
> + *doorbell_off = kfd->doorbell_base_dw_offset + inx;
>
> pr_debug("Get kernel queue doorbell\n"
> " doorbell offset == 0x%08X\n"
> @@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
> }
> }
>
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id)
> {
> /*
> - * doorbell_id_offset accounts for doorbells taken by KGD.
> + * doorbell_base_dw_offset accounts for doorbells taken by KGD.
> * index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
> * the process's doorbells. The offset returned is in dword
> * units regardless of the ASIC-dependent doorbell size.
> */
> - return kfd->doorbell_id_offset +
> + return kfd->doorbell_base_dw_offset +
> process->doorbell_index
> * kfd_doorbell_process_slice(kfd) / sizeof(u32) +
> doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 62db4d20ed32..7c561c98f2e2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -238,9 +238,9 @@ struct kfd_dev {
> * KFD. It is aligned for mapping
> * into user mode
> */
> - size_t doorbell_id_offset; /* Doorbell offset (from KFD doorbell
> - * to HW doorbell, GFX reserved some
> - * at the start)
> + size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
> + * doorbell to PCI doorbell bar,
> + * GFX reserved some at the start)
This is still a bit convoluted and sounds backwards. I suggest this wording:
Offset from the start of the PCI doorbell BAR to the first KFD
doorbell in dwords
Regards,
Felix
> */
> u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
> * page used by kernel queue
> @@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 __iomem *db_addr);
> u32 read_kernel_doorbell(u32 __iomem *db);
> void write_kernel_doorbell(void __iomem *db, u32 value);
> void write_kernel_doorbell64(void __iomem *db, u64 value);
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id);
> phys_addr_t kfd_get_process_doorbells(struct kfd_dev *dev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords
@ 2019-11-11 20:29 ` Felix Kuehling
0 siblings, 0 replies; 14+ messages in thread
From: Felix Kuehling @ 2019-11-11 20:29 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx
On 2019-11-01 16:10, Zhao, Yong wrote:
> Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
I agree with the name changes. One suggestion for a comment inline. With
that fixed, this patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 14 +++++++-------
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> 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 984c2f2b24b6..4503fb26fe5b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
> }
>
> q->properties.doorbell_off =
> - kfd_doorbell_id_to_offset(dev, q->process,
> + kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
> q->doorbell_id);
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> index ebe79bf00145..f904355c44a1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
> @@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
> doorbell_start_offset;
>
> - kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
> + kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
>
> kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
> kfd_doorbell_process_slice(kfd));
> @@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
> pr_debug("doorbell base == 0x%08lX\n",
> (uintptr_t)kfd->doorbell_base);
>
> - pr_debug("doorbell_id_offset == 0x%08lX\n",
> - kfd->doorbell_id_offset);
> + pr_debug("doorbell_base_dw_offset == 0x%08lX\n",
> + kfd->doorbell_base_dw_offset);
>
> pr_debug("doorbell_process_limit == 0x%08lX\n",
> doorbell_process_limit);
> @@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
> * Calculating the kernel doorbell offset using the first
> * doorbell page.
> */
> - *doorbell_off = kfd->doorbell_id_offset + inx;
> + *doorbell_off = kfd->doorbell_base_dw_offset + inx;
>
> pr_debug("Get kernel queue doorbell\n"
> " doorbell offset == 0x%08X\n"
> @@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
> }
> }
>
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id)
> {
> /*
> - * doorbell_id_offset accounts for doorbells taken by KGD.
> + * doorbell_base_dw_offset accounts for doorbells taken by KGD.
> * index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
> * the process's doorbells. The offset returned is in dword
> * units regardless of the ASIC-dependent doorbell size.
> */
> - return kfd->doorbell_id_offset +
> + return kfd->doorbell_base_dw_offset +
> process->doorbell_index
> * kfd_doorbell_process_slice(kfd) / sizeof(u32) +
> doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 62db4d20ed32..7c561c98f2e2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -238,9 +238,9 @@ struct kfd_dev {
> * KFD. It is aligned for mapping
> * into user mode
> */
> - size_t doorbell_id_offset; /* Doorbell offset (from KFD doorbell
> - * to HW doorbell, GFX reserved some
> - * at the start)
> + size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
> + * doorbell to PCI doorbell bar,
> + * GFX reserved some at the start)
This is still a bit convoluted and sounds backwards. I suggest this wording:
Offset from the start of the PCI doorbell BAR to the first KFD
doorbell in dwords
Regards,
Felix
> */
> u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
> * page used by kernel queue
> @@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 __iomem *db_addr);
> u32 read_kernel_doorbell(u32 __iomem *db);
> void write_kernel_doorbell(void __iomem *db, u32 value);
> void write_kernel_doorbell64(void __iomem *db, u64 value);
> -unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
> +unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
> struct kfd_process *process,
> unsigned int doorbell_id);
> phys_addr_t kfd_get_process_doorbells(struct kfd_dev *dev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-11 20:43 ` Felix Kuehling
0 siblings, 0 replies; 14+ messages in thread
From: Felix Kuehling @ 2019-11-11 20:43 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 5585 bytes --]
On 2019-11-01 16:10, Zhao, Yong wrote:
> dorbell_off in the queue properties is mainly used for the doorbell dw
> offset in pci bar. We should not set it to the doorbell byte offset in
> process doorbell pages. This makes the code much easier to read.
I kind of agree. I think what's confusing is that the queue_properties
structure is used for two different purposes.
1. For storing queue properties provided by user mode through KFD ioctls
2. A subset of struct queue passed to mqd_manager and elsewhere (that's
why some driver state is creeping into it)
Maybe a follow-up could cleanly separate the queue properties from the
queue driver state. That would probably change some internal interfaces
to use struct queue instead of queue_properties.
Anyway, this patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
> Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
> Signed-off-by: Yong Zhao <Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
> 4 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index d9e36dbf13d5..b91993753b82 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
> unsigned int queue_id;
> struct kfd_process_device *pdd;
> struct queue_properties q_properties;
> + uint32_t doorbell_offset_in_process = 0;
>
> memset(&q_properties, 0, sizeof(struct queue_properties));
>
> @@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
> p->pasid,
> dev->id);
>
> - err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
> + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
> + &doorbell_offset_in_process);
> if (err != 0)
> goto err_create_queue;
>
> @@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
> args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
> args->doorbell_offset <<= PAGE_SHIFT;
> if (KFD_IS_SOC15(dev->device_info->asic_family))
> - /* On SOC15 ASICs, doorbell allocation must be
> - * per-device, and independent from the per-process
> - * queue_id. Return the doorbell offset within the
> - * doorbell aperture to user mode.
> + /* On SOC15 ASICs, include the doorbell offset within the
> + * process doorbell frame, which could be 1 page or 2 pages.
> */
> - args->doorbell_offset |= q_properties.doorbell_off;
> + args->doorbell_offset |= doorbell_offset_in_process;
>
> mutex_unlock(&p->mutex);
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> index d59f2cd056c6..1d33c4f25263 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> @@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
> properties.type = KFD_QUEUE_TYPE_DIQ;
>
> status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
> - &properties, &qid);
> + &properties, &qid, NULL);
>
> if (status) {
> pr_err("Failed to create DIQ\n");
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 7c561c98f2e2..66bae8f2dad1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
> struct kfd_dev *dev,
> struct file *f,
> struct queue_properties *properties,
> - unsigned int *qid);
> + unsigned int *qid,
> + uint32_t *p_doorbell_offset_in_process);
> int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
> int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
> struct queue_properties *p);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 8509814a6ff0..48185d2957e9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
> struct kfd_dev *dev,
> struct file *f,
> struct queue_properties *properties,
> - unsigned int *qid)
> + unsigned int *qid,
> + uint32_t *p_doorbell_offset_in_process)
> {
> int retval;
> struct kfd_process_device *pdd;
> @@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
> /* Return the doorbell offset within the doorbell page
> * to the caller so it can be passed up to user mode
> * (in bytes).
> + * There are always 1024 doorbells per process, so in case
> + * of 8-byte doorbells, there are two doorbell pages per
> + * process.
> */
> - properties->doorbell_off =
> *p_doorbell_offset_in_process =
You need a NULL pointer check here because you call this with a NULL
pointer from the DIQ code.
> (q->properties.doorbell_off * sizeof(uint32_t)) &
> (kfd_doorbell_process_slice(dev) - 1);
>
[-- Attachment #1.2: Type: text/html, Size: 6726 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-11 20:43 ` Felix Kuehling
0 siblings, 0 replies; 14+ messages in thread
From: Felix Kuehling @ 2019-11-11 20:43 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 5543 bytes --]
On 2019-11-01 16:10, Zhao, Yong wrote:
> dorbell_off in the queue properties is mainly used for the doorbell dw
> offset in pci bar. We should not set it to the doorbell byte offset in
> process doorbell pages. This makes the code much easier to read.
I kind of agree. I think what's confusing is that the queue_properties
structure is used for two different purposes.
1. For storing queue properties provided by user mode through KFD ioctls
2. A subset of struct queue passed to mqd_manager and elsewhere (that's
why some driver state is creeping into it)
Maybe a follow-up could cleanly separate the queue properties from the
queue driver state. That would probably change some internal interfaces
to use struct queue instead of queue_properties.
Anyway, this patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
> 4 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index d9e36dbf13d5..b91993753b82 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
> unsigned int queue_id;
> struct kfd_process_device *pdd;
> struct queue_properties q_properties;
> + uint32_t doorbell_offset_in_process = 0;
>
> memset(&q_properties, 0, sizeof(struct queue_properties));
>
> @@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
> p->pasid,
> dev->id);
>
> - err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
> + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
> + &doorbell_offset_in_process);
> if (err != 0)
> goto err_create_queue;
>
> @@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
> args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
> args->doorbell_offset <<= PAGE_SHIFT;
> if (KFD_IS_SOC15(dev->device_info->asic_family))
> - /* On SOC15 ASICs, doorbell allocation must be
> - * per-device, and independent from the per-process
> - * queue_id. Return the doorbell offset within the
> - * doorbell aperture to user mode.
> + /* On SOC15 ASICs, include the doorbell offset within the
> + * process doorbell frame, which could be 1 page or 2 pages.
> */
> - args->doorbell_offset |= q_properties.doorbell_off;
> + args->doorbell_offset |= doorbell_offset_in_process;
>
> mutex_unlock(&p->mutex);
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> index d59f2cd056c6..1d33c4f25263 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> @@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
> properties.type = KFD_QUEUE_TYPE_DIQ;
>
> status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
> - &properties, &qid);
> + &properties, &qid, NULL);
>
> if (status) {
> pr_err("Failed to create DIQ\n");
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 7c561c98f2e2..66bae8f2dad1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
> struct kfd_dev *dev,
> struct file *f,
> struct queue_properties *properties,
> - unsigned int *qid);
> + unsigned int *qid,
> + uint32_t *p_doorbell_offset_in_process);
> int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
> int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
> struct queue_properties *p);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 8509814a6ff0..48185d2957e9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
> struct kfd_dev *dev,
> struct file *f,
> struct queue_properties *properties,
> - unsigned int *qid)
> + unsigned int *qid,
> + uint32_t *p_doorbell_offset_in_process)
> {
> int retval;
> struct kfd_process_device *pdd;
> @@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
> /* Return the doorbell offset within the doorbell page
> * to the caller so it can be passed up to user mode
> * (in bytes).
> + * There are always 1024 doorbells per process, so in case
> + * of 8-byte doorbells, there are two doorbell pages per
> + * process.
> */
> - properties->doorbell_off =
> *p_doorbell_offset_in_process =
You need a NULL pointer check here because you call this with a NULL
pointer from the DIQ code.
> (q->properties.doorbell_off * sizeof(uint32_t)) &
> (kfd_doorbell_process_slice(dev) - 1);
>
[-- Attachment #1.2: Type: text/html, Size: 6579 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-11 20:51 ` Felix Kuehling
0 siblings, 0 replies; 14+ messages in thread
From: Felix Kuehling @ 2019-11-11 20:51 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 6125 bytes --]
On 2019-11-11 15:43, Felix Kuehling wrote:
> On 2019-11-01 16:10, Zhao, Yong wrote:
>> dorbell_off in the queue properties is mainly used for the doorbell dw
>> offset in pci bar. We should not set it to the doorbell byte offset in
>> process doorbell pages. This makes the code much easier to read.
>
> I kind of agree. I think what's confusing is that the queue_properties
> structure is used for two different purposes.
>
> 1. For storing queue properties provided by user mode through KFD ioctls
> 2. A subset of struct queue passed to mqd_manager and elsewhere
> (that's why some driver state is creeping into it)
>
> Maybe a follow-up could cleanly separate the queue properties from the
> queue driver state. That would probably change some internal
> interfaces to use struct queue instead of queue_properties.
>
> Anyway, this patch is
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
>
I pointed out a missing NULL pointer check inline near the end of the
patch. I should have mentioned it here. Please fix that before you submit.
Thanks,
Felix
>> Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
>> Signed-off-by: Yong Zhao<Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
>> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
>> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
>> 4 files changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> index d9e36dbf13d5..b91993753b82 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> @@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>> unsigned int queue_id;
>> struct kfd_process_device *pdd;
>> struct queue_properties q_properties;
>> + uint32_t doorbell_offset_in_process = 0;
>>
>> memset(&q_properties, 0, sizeof(struct queue_properties));
>>
>> @@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>> p->pasid,
>> dev->id);
>>
>> - err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
>> + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
>> + &doorbell_offset_in_process);
>> if (err != 0)
>> goto err_create_queue;
>>
>> @@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>> args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
>> args->doorbell_offset <<= PAGE_SHIFT;
>> if (KFD_IS_SOC15(dev->device_info->asic_family))
>> - /* On SOC15 ASICs, doorbell allocation must be
>> - * per-device, and independent from the per-process
>> - * queue_id. Return the doorbell offset within the
>> - * doorbell aperture to user mode.
>> + /* On SOC15 ASICs, include the doorbell offset within the
>> + * process doorbell frame, which could be 1 page or 2 pages.
>> */
>> - args->doorbell_offset |= q_properties.doorbell_off;
>> + args->doorbell_offset |= doorbell_offset_in_process;
>>
>> mutex_unlock(&p->mutex);
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> index d59f2cd056c6..1d33c4f25263 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> @@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
>> properties.type = KFD_QUEUE_TYPE_DIQ;
>>
>> status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
>> - &properties, &qid);
>> + &properties, &qid, NULL);
>>
>> if (status) {
>> pr_err("Failed to create DIQ\n");
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 7c561c98f2e2..66bae8f2dad1 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> struct kfd_dev *dev,
>> struct file *f,
>> struct queue_properties *properties,
>> - unsigned int *qid);
>> + unsigned int *qid,
>> + uint32_t *p_doorbell_offset_in_process);
>> int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
>> int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
>> struct queue_properties *p);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> index 8509814a6ff0..48185d2957e9 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> @@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> struct kfd_dev *dev,
>> struct file *f,
>> struct queue_properties *properties,
>> - unsigned int *qid)
>> + unsigned int *qid,
>> + uint32_t *p_doorbell_offset_in_process)
>> {
>> int retval;
>> struct kfd_process_device *pdd;
>> @@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> /* Return the doorbell offset within the doorbell page
>> * to the caller so it can be passed up to user mode
>> * (in bytes).
>> + * There are always 1024 doorbells per process, so in case
>> + * of 8-byte doorbells, there are two doorbell pages per
>> + * process.
>> */
>> - properties->doorbell_off =
>> *p_doorbell_offset_in_process =
>
> You need a NULL pointer check here because you call this with a NULL
> pointer from the DIQ code.
>
>
>> (q->properties.doorbell_off * sizeof(uint32_t)) &
>> (kfd_doorbell_process_slice(dev) - 1);
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 7959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-11 20:51 ` Felix Kuehling
0 siblings, 0 replies; 14+ messages in thread
From: Felix Kuehling @ 2019-11-11 20:51 UTC (permalink / raw)
To: Zhao, Yong, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 6055 bytes --]
On 2019-11-11 15:43, Felix Kuehling wrote:
> On 2019-11-01 16:10, Zhao, Yong wrote:
>> dorbell_off in the queue properties is mainly used for the doorbell dw
>> offset in pci bar. We should not set it to the doorbell byte offset in
>> process doorbell pages. This makes the code much easier to read.
>
> I kind of agree. I think what's confusing is that the queue_properties
> structure is used for two different purposes.
>
> 1. For storing queue properties provided by user mode through KFD ioctls
> 2. A subset of struct queue passed to mqd_manager and elsewhere
> (that's why some driver state is creeping into it)
>
> Maybe a follow-up could cleanly separate the queue properties from the
> queue driver state. That would probably change some internal
> interfaces to use struct queue instead of queue_properties.
>
> Anyway, this patch is
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
I pointed out a missing NULL pointer check inline near the end of the
patch. I should have mentioned it here. Please fix that before you submit.
Thanks,
Felix
>> Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
>> Signed-off-by: Yong Zhao<Yong.Zhao@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
>> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
>> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
>> 4 files changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> index d9e36dbf13d5..b91993753b82 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> @@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>> unsigned int queue_id;
>> struct kfd_process_device *pdd;
>> struct queue_properties q_properties;
>> + uint32_t doorbell_offset_in_process = 0;
>>
>> memset(&q_properties, 0, sizeof(struct queue_properties));
>>
>> @@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>> p->pasid,
>> dev->id);
>>
>> - err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
>> + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
>> + &doorbell_offset_in_process);
>> if (err != 0)
>> goto err_create_queue;
>>
>> @@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>> args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
>> args->doorbell_offset <<= PAGE_SHIFT;
>> if (KFD_IS_SOC15(dev->device_info->asic_family))
>> - /* On SOC15 ASICs, doorbell allocation must be
>> - * per-device, and independent from the per-process
>> - * queue_id. Return the doorbell offset within the
>> - * doorbell aperture to user mode.
>> + /* On SOC15 ASICs, include the doorbell offset within the
>> + * process doorbell frame, which could be 1 page or 2 pages.
>> */
>> - args->doorbell_offset |= q_properties.doorbell_off;
>> + args->doorbell_offset |= doorbell_offset_in_process;
>>
>> mutex_unlock(&p->mutex);
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> index d59f2cd056c6..1d33c4f25263 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> @@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
>> properties.type = KFD_QUEUE_TYPE_DIQ;
>>
>> status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
>> - &properties, &qid);
>> + &properties, &qid, NULL);
>>
>> if (status) {
>> pr_err("Failed to create DIQ\n");
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 7c561c98f2e2..66bae8f2dad1 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> struct kfd_dev *dev,
>> struct file *f,
>> struct queue_properties *properties,
>> - unsigned int *qid);
>> + unsigned int *qid,
>> + uint32_t *p_doorbell_offset_in_process);
>> int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
>> int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
>> struct queue_properties *p);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> index 8509814a6ff0..48185d2957e9 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> @@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> struct kfd_dev *dev,
>> struct file *f,
>> struct queue_properties *properties,
>> - unsigned int *qid)
>> + unsigned int *qid,
>> + uint32_t *p_doorbell_offset_in_process)
>> {
>> int retval;
>> struct kfd_process_device *pdd;
>> @@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> /* Return the doorbell offset within the doorbell page
>> * to the caller so it can be passed up to user mode
>> * (in bytes).
>> + * There are always 1024 doorbells per process, so in case
>> + * of 8-byte doorbells, there are two doorbell pages per
>> + * process.
>> */
>> - properties->doorbell_off =
>> *p_doorbell_offset_in_process =
>
> You need a NULL pointer check here because you call this with a NULL
> pointer from the DIQ code.
>
>
>> (q->properties.doorbell_off * sizeof(uint32_t)) &
>> (kfd_doorbell_process_slice(dev) - 1);
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 7714 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-11 23:06 ` Yong Zhao
0 siblings, 0 replies; 14+ messages in thread
From: Yong Zhao @ 2019-11-11 23:06 UTC (permalink / raw)
To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 6765 bytes --]
The NULL pointer is not an issue, because for DIQ, the if (q) condition,
which guards the section but is now shown, will never be satisfied.
Anyway, I still added the NULL pointer check.
With that, I have pushed the change.
Yong
On 2019-11-11 3:51 p.m., Felix Kuehling wrote:
> On 2019-11-11 15:43, Felix Kuehling wrote:
>> On 2019-11-01 16:10, Zhao, Yong wrote:
>>> dorbell_off in the queue properties is mainly used for the doorbell dw
>>> offset in pci bar. We should not set it to the doorbell byte offset in
>>> process doorbell pages. This makes the code much easier to read.
>>
>> I kind of agree. I think what's confusing is that the
>> queue_properties structure is used for two different purposes.
>>
>> 1. For storing queue properties provided by user mode through KFD ioctls
>> 2. A subset of struct queue passed to mqd_manager and elsewhere
>> (that's why some driver state is creeping into it)
>>
>> Maybe a follow-up could cleanly separate the queue properties from
>> the queue driver state. That would probably change some internal
>> interfaces to use struct queue instead of queue_properties.
>>
>> Anyway, this patch is
>>
>> Reviewed-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
>>
> I pointed out a missing NULL pointer check inline near the end of the
> patch. I should have mentioned it here. Please fix that before you submit.
>
> Thanks,
> Felix
>
>
>>> Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
>>> Signed-off-by: Yong Zhao<Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
>>> ---
>>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
>>> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
>>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
>>> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
>>> 4 files changed, 15 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> index d9e36dbf13d5..b91993753b82 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> @@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>>> unsigned int queue_id;
>>> struct kfd_process_device *pdd;
>>> struct queue_properties q_properties;
>>> + uint32_t doorbell_offset_in_process = 0;
>>>
>>> memset(&q_properties, 0, sizeof(struct queue_properties));
>>>
>>> @@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>>> p->pasid,
>>> dev->id);
>>>
>>> - err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
>>> + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
>>> + &doorbell_offset_in_process);
>>> if (err != 0)
>>> goto err_create_queue;
>>>
>>> @@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>>> args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
>>> args->doorbell_offset <<= PAGE_SHIFT;
>>> if (KFD_IS_SOC15(dev->device_info->asic_family))
>>> - /* On SOC15 ASICs, doorbell allocation must be
>>> - * per-device, and independent from the per-process
>>> - * queue_id. Return the doorbell offset within the
>>> - * doorbell aperture to user mode.
>>> + /* On SOC15 ASICs, include the doorbell offset within the
>>> + * process doorbell frame, which could be 1 page or 2 pages.
>>> */
>>> - args->doorbell_offset |= q_properties.doorbell_off;
>>> + args->doorbell_offset |= doorbell_offset_in_process;
>>>
>>> mutex_unlock(&p->mutex);
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> index d59f2cd056c6..1d33c4f25263 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> @@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
>>> properties.type = KFD_QUEUE_TYPE_DIQ;
>>>
>>> status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
>>> - &properties, &qid);
>>> + &properties, &qid, NULL);
>>>
>>> if (status) {
>>> pr_err("Failed to create DIQ\n");
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> index 7c561c98f2e2..66bae8f2dad1 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> @@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>>> struct kfd_dev *dev,
>>> struct file *f,
>>> struct queue_properties *properties,
>>> - unsigned int *qid);
>>> + unsigned int *qid,
>>> + uint32_t *p_doorbell_offset_in_process);
>>> int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
>>> int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
>>> struct queue_properties *p);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> index 8509814a6ff0..48185d2957e9 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> @@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>>> struct kfd_dev *dev,
>>> struct file *f,
>>> struct queue_properties *properties,
>>> - unsigned int *qid)
>>> + unsigned int *qid,
>>> + uint32_t *p_doorbell_offset_in_process)
>>> {
>>> int retval;
>>> struct kfd_process_device *pdd;
>>> @@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>>> /* Return the doorbell offset within the doorbell page
>>> * to the caller so it can be passed up to user mode
>>> * (in bytes).
>>> + * There are always 1024 doorbells per process, so in case
>>> + * of 8-byte doorbells, there are two doorbell pages per
>>> + * process.
>>> */
>>> - properties->doorbell_off =
>>> *p_doorbell_offset_in_process =
>>
>> You need a NULL pointer check here because you call this with a NULL
>> pointer from the DIQ code.
>>
It is not an issue, because for DIQ, the if (q) condition, which guards
this section but is now shown, will never be satisfied. Anyway, I still
added the NULL pointer check.
With that, I have pushed the change.
>>
>>> (q->properties.doorbell_off * sizeof(uint32_t)) &
>>> (kfd_doorbell_process_slice(dev) - 1);
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 9163 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages
@ 2019-11-11 23:06 ` Yong Zhao
0 siblings, 0 replies; 14+ messages in thread
From: Yong Zhao @ 2019-11-11 23:06 UTC (permalink / raw)
To: Felix Kuehling, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 6695 bytes --]
The NULL pointer is not an issue, because for DIQ, the if (q) condition,
which guards the section but is now shown, will never be satisfied.
Anyway, I still added the NULL pointer check.
With that, I have pushed the change.
Yong
On 2019-11-11 3:51 p.m., Felix Kuehling wrote:
> On 2019-11-11 15:43, Felix Kuehling wrote:
>> On 2019-11-01 16:10, Zhao, Yong wrote:
>>> dorbell_off in the queue properties is mainly used for the doorbell dw
>>> offset in pci bar. We should not set it to the doorbell byte offset in
>>> process doorbell pages. This makes the code much easier to read.
>>
>> I kind of agree. I think what's confusing is that the
>> queue_properties structure is used for two different purposes.
>>
>> 1. For storing queue properties provided by user mode through KFD ioctls
>> 2. A subset of struct queue passed to mqd_manager and elsewhere
>> (that's why some driver state is creeping into it)
>>
>> Maybe a follow-up could cleanly separate the queue properties from
>> the queue driver state. That would probably change some internal
>> interfaces to use struct queue instead of queue_properties.
>>
>> Anyway, this patch is
>>
>> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>
> I pointed out a missing NULL pointer check inline near the end of the
> patch. I should have mentioned it here. Please fix that before you submit.
>
> Thanks,
> Felix
>
>
>>> Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
>>> Signed-off-by: Yong Zhao<Yong.Zhao@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++++++------
>>> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +-
>>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ++-
>>> .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 8 ++++++--
>>> 4 files changed, 15 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> index d9e36dbf13d5..b91993753b82 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> @@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>>> unsigned int queue_id;
>>> struct kfd_process_device *pdd;
>>> struct queue_properties q_properties;
>>> + uint32_t doorbell_offset_in_process = 0;
>>>
>>> memset(&q_properties, 0, sizeof(struct queue_properties));
>>>
>>> @@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>>> p->pasid,
>>> dev->id);
>>>
>>> - err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
>>> + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
>>> + &doorbell_offset_in_process);
>>> if (err != 0)
>>> goto err_create_queue;
>>>
>>> @@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>>> args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
>>> args->doorbell_offset <<= PAGE_SHIFT;
>>> if (KFD_IS_SOC15(dev->device_info->asic_family))
>>> - /* On SOC15 ASICs, doorbell allocation must be
>>> - * per-device, and independent from the per-process
>>> - * queue_id. Return the doorbell offset within the
>>> - * doorbell aperture to user mode.
>>> + /* On SOC15 ASICs, include the doorbell offset within the
>>> + * process doorbell frame, which could be 1 page or 2 pages.
>>> */
>>> - args->doorbell_offset |= q_properties.doorbell_off;
>>> + args->doorbell_offset |= doorbell_offset_in_process;
>>>
>>> mutex_unlock(&p->mutex);
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> index d59f2cd056c6..1d33c4f25263 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> @@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
>>> properties.type = KFD_QUEUE_TYPE_DIQ;
>>>
>>> status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
>>> - &properties, &qid);
>>> + &properties, &qid, NULL);
>>>
>>> if (status) {
>>> pr_err("Failed to create DIQ\n");
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> index 7c561c98f2e2..66bae8f2dad1 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> @@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>>> struct kfd_dev *dev,
>>> struct file *f,
>>> struct queue_properties *properties,
>>> - unsigned int *qid);
>>> + unsigned int *qid,
>>> + uint32_t *p_doorbell_offset_in_process);
>>> int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
>>> int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
>>> struct queue_properties *p);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> index 8509814a6ff0..48185d2957e9 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> @@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>>> struct kfd_dev *dev,
>>> struct file *f,
>>> struct queue_properties *properties,
>>> - unsigned int *qid)
>>> + unsigned int *qid,
>>> + uint32_t *p_doorbell_offset_in_process)
>>> {
>>> int retval;
>>> struct kfd_process_device *pdd;
>>> @@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>>> /* Return the doorbell offset within the doorbell page
>>> * to the caller so it can be passed up to user mode
>>> * (in bytes).
>>> + * There are always 1024 doorbells per process, so in case
>>> + * of 8-byte doorbells, there are two doorbell pages per
>>> + * process.
>>> */
>>> - properties->doorbell_off =
>>> *p_doorbell_offset_in_process =
>>
>> You need a NULL pointer check here because you call this with a NULL
>> pointer from the DIQ code.
>>
It is not an issue, because for DIQ, the if (q) condition, which guards
this section but is now shown, will never be satisfied. Anyway, I still
added the NULL pointer check.
With that, I have pushed the change.
>>
>>> (q->properties.doorbell_off * sizeof(uint32_t)) &
>>> (kfd_doorbell_process_slice(dev) - 1);
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 8855 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-11-11 23:06 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-01 20:10 [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords Zhao, Yong
2019-11-01 20:10 ` Zhao, Yong
[not found] ` <20191101201016.5973-1-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-11-01 20:10 ` [PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages Zhao, Yong
2019-11-01 20:10 ` Zhao, Yong
[not found] ` <20191101201016.5973-2-Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
2019-11-11 20:43 ` Felix Kuehling
2019-11-11 20:43 ` Felix Kuehling
[not found] ` <a21e8321-51bf-f840-f891-2335be774131-5C7GfCeVMHo@public.gmane.org>
2019-11-11 20:51 ` Felix Kuehling
2019-11-11 20:51 ` Felix Kuehling
[not found] ` <78f9e381-e490-5555-c84d-ca76dcee7c83-5C7GfCeVMHo@public.gmane.org>
2019-11-11 23:06 ` Yong Zhao
2019-11-11 23:06 ` Yong Zhao
2019-11-11 18:04 ` [PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords Yong Zhao
2019-11-11 18:04 ` Yong Zhao
2019-11-11 20:29 ` Felix Kuehling
2019-11-11 20:29 ` Felix Kuehling
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.