<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
CI just refers to the dGPUs (bonaire and hawaii), the CIK refers to the whole family (CI dGPUs, Indus (kaveri platform), and Kerala (kabini/mullins platform).</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Alex<br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Russell, Kent <Kent.Russell@amd.com><br>
<b>Sent:</b> Wednesday, November 13, 2019 8:28 AM<br>
<b>To:</b> Zhao, Yong <Yong.Zhao@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Zhao, Yong <Yong.Zhao@amd.com><br>
<b>Subject:</b> RE: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Should we use "CI" instead of "CIK" in the comments? I thought CIK was a short form for CI kickers, while CI encompasses all CI ASICs. Even though we had _cik as the suffix for most of the CI stuff. Just wondering about accuracy.<br>
<br>
 Kent<br>
<br>
-----Original Message-----<br>
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Yong Zhao<br>
Sent: Tuesday, November 12, 2019 5:19 PM<br>
To: amd-gfx@lists.freedesktop.org<br>
Cc: Zhao, Yong <Yong.Zhao@amd.com><br>
Subject: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue<br>
<br>
The ops_asic_specific function pointers are actually quite generic after using a simple if condition. Eliminate it by code refactoring.<br>
<br>
Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08<br>
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com><br>
---<br>
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ++++++++-----------  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 -----------  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --------------<br>
 4 files changed, 26 insertions(+), 125 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c<br>
index a750b1d110eb..59ee9053498c 100644<br>
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c<br>
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c<br>
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,<br>
         kq->pq_kernel_addr = kq->pq->cpu_ptr;<br>
         kq->pq_gpu_addr = kq->pq->gpu_addr;<br>
 <br>
-       retval = kq->ops_asic_specific.initialize(kq, dev, type, queue_size);<br>
-       if (!retval)<br>
-               goto err_eop_allocate_vidmem;<br>
+       /* For CIK family asics, kq->eop_mem is not needed */<br>
+       if (dev->device_info->asic_family > CHIP_HAWAII) {<br>
+               retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem);<br>
+               if (retval != 0)<br>
+                       goto err_eop_allocate_vidmem;<br>
+<br>
+               kq->eop_gpu_addr = kq->eop_mem->gpu_addr;<br>
+               kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;<br>
+<br>
+               memset(kq->eop_kernel_addr, 0, PAGE_SIZE);<br>
+       }<br>
 <br>
         retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),<br>
                                         &kq->rptr_mem);<br>
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)<br>
 <br>
         kfd_gtt_sa_free(kq->dev, kq->rptr_mem);<br>
         kfd_gtt_sa_free(kq->dev, kq->wptr_mem);<br>
-       kq->ops_asic_specific.uninitialize(kq);<br>
+<br>
+       /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()<br>
+        * is able to handle NULL properly.<br>
+        */<br>
+       kfd_gtt_sa_free(kq->dev, kq->eop_mem);<br>
+<br>
         kfd_gtt_sa_free(kq->dev, kq->pq);<br>
         kfd_release_kernel_doorbell(kq->dev,<br>
                                         kq->queue->properties.doorbell_ptr);<br>
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)<br>
         }<br>
         pr_debug("\n");<br>
 #endif<br>
-<br>
-       kq->ops_asic_specific.submit_packet(kq);<br>
+       if (kq->dev->device_info->doorbell_size == 8) {<br>
+               *kq->wptr64_kernel = kq->pending_wptr64;<br>
+               write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,<br>
+                                       kq->pending_wptr64);<br>
+       } else {<br>
+               *kq->wptr_kernel = kq->pending_wptr;<br>
+               write_kernel_doorbell(kq->queue->properties.doorbell_ptr,<br>
+                                       kq->pending_wptr);<br>
+       }<br>
 }<br>
 <br>
 static void rollback_packet(struct kernel_queue *kq) @@ -310,42 +330,11 @@ struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,<br>
         kq->ops.submit_packet = submit_packet;<br>
         kq->ops.rollback_packet = rollback_packet;<br>
 <br>
-       switch (dev->device_info->asic_family) {<br>
-       case CHIP_KAVERI:<br>
-       case CHIP_HAWAII:<br>
-       case CHIP_CARRIZO:<br>
-       case CHIP_TONGA:<br>
-       case CHIP_FIJI:<br>
-       case CHIP_POLARIS10:<br>
-       case CHIP_POLARIS11:<br>
-       case CHIP_POLARIS12:<br>
-       case CHIP_VEGAM:<br>
-               kernel_queue_init_vi(&kq->ops_asic_specific);<br>
-               break;<br>
-<br>
-       case CHIP_VEGA10:<br>
-       case CHIP_VEGA12:<br>
-       case CHIP_VEGA20:<br>
-       case CHIP_RAVEN:<br>
-       case CHIP_RENOIR:<br>
-       case CHIP_ARCTURUS:<br>
-       case CHIP_NAVI10:<br>
-       case CHIP_NAVI12:<br>
-       case CHIP_NAVI14:<br>
-               kernel_queue_init_v9(&kq->ops_asic_specific);<br>
-               break;<br>
-       default:<br>
-               WARN(1, "Unexpected ASIC family %u",<br>
-                    dev->device_info->asic_family);<br>
-               goto out_free;<br>
-       }<br>
-<br>
         if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))<br>
                 return kq;<br>
 <br>
         pr_err("Failed to init kernel queue\n");<br>
 <br>
-out_free:<br>
         kfree(kq);<br>
         return NULL;<br>
 }<br>
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h<br>
index a9a35897d8b7..475e9499c0af 100644<br>
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h<br>
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h<br>
@@ -66,7 +66,6 @@ struct kernel_queue_ops {<br>
 <br>
 struct kernel_queue {<br>
         struct kernel_queue_ops ops;<br>
-       struct kernel_queue_ops ops_asic_specific;<br>
 <br>
         /* data */<br>
         struct kfd_dev          *dev;<br>
@@ -99,7 +98,4 @@ struct kernel_queue {<br>
         struct list_head        list;<br>
 };<br>
 <br>
-void kernel_queue_init_vi(struct kernel_queue_ops *ops); -void kernel_queue_init_v9(struct kernel_queue_ops *ops);<br>
-<br>
 #endif /* KFD_KERNEL_QUEUE_H_ */<br>
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c<br>
index 9e0eaf446bab..2de01009f1b6 100644<br>
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c<br>
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c<br>
@@ -27,42 +27,6 @@<br>
 #include "kfd_pm4_opcodes.h"<br>
 #include "gc/gc_10_1_0_sh_mask.h"<br>
 <br>
-static bool initialize_v9(struct kernel_queue *kq, struct kfd_dev *dev,<br>
-                       enum kfd_queue_type type, unsigned int queue_size)<br>
-{<br>
-       int retval;<br>
-<br>
-       retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem);<br>
-       if (retval)<br>
-               return false;<br>
-<br>
-       kq->eop_gpu_addr = kq->eop_mem->gpu_addr;<br>
-       kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;<br>
-<br>
-       memset(kq->eop_kernel_addr, 0, PAGE_SIZE);<br>
-<br>
-       return true;<br>
-}<br>
-<br>
-static void uninitialize_v9(struct kernel_queue *kq) -{<br>
-       kfd_gtt_sa_free(kq->dev, kq->eop_mem);<br>
-}<br>
-<br>
-static void submit_packet_v9(struct kernel_queue *kq) -{<br>
-       *kq->wptr64_kernel = kq->pending_wptr64;<br>
-       write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,<br>
-                               kq->pending_wptr64);<br>
-}<br>
-<br>
-void kernel_queue_init_v9(struct kernel_queue_ops *ops) -{<br>
-       ops->initialize = initialize_v9;<br>
-       ops->uninitialize = uninitialize_v9;<br>
-       ops->submit_packet = submit_packet_v9;<br>
-}<br>
-<br>
 static int pm_map_process_v9(struct packet_manager *pm,<br>
                 uint32_t *buffer, struct qcm_process_device *qpd)  { diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c<br>
index 64f13f34d819..bed4d0ccb6b1 100644<br>
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c<br>
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c<br>
@@ -26,54 +26,6 @@<br>
 #include "kfd_pm4_headers_vi.h"<br>
 #include "kfd_pm4_opcodes.h"<br>
 <br>
-static bool initialize_vi(struct kernel_queue *kq, struct kfd_dev *dev,<br>
-                       enum kfd_queue_type type, unsigned int queue_size);<br>
-static void uninitialize_vi(struct kernel_queue *kq); -static void submit_packet_vi(struct kernel_queue *kq);<br>
-<br>
-void kernel_queue_init_vi(struct kernel_queue_ops *ops) -{<br>
-       ops->initialize = initialize_vi;<br>
-       ops->uninitialize = uninitialize_vi;<br>
-       ops->submit_packet = submit_packet_vi;<br>
-}<br>
-<br>
-static bool initialize_vi(struct kernel_queue *kq, struct kfd_dev *dev,<br>
-                       enum kfd_queue_type type, unsigned int queue_size)<br>
-{<br>
-       int retval;<br>
-<br>
-       /*For CIK family asics, kq->eop_mem is not needed */<br>
-       if (dev->device_info->asic_family <= CHIP_HAWAII)<br>
-               return true;<br>
-<br>
-       retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem);<br>
-       if (retval != 0)<br>
-               return false;<br>
-<br>
-       kq->eop_gpu_addr = kq->eop_mem->gpu_addr;<br>
-       kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;<br>
-<br>
-       memset(kq->eop_kernel_addr, 0, PAGE_SIZE);<br>
-<br>
-       return true;<br>
-}<br>
-<br>
-static void uninitialize_vi(struct kernel_queue *kq) -{<br>
-       /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()<br>
-        * is able to handle NULL properly.<br>
-        */<br>
-       kfd_gtt_sa_free(kq->dev, kq->eop_mem);<br>
-}<br>
-<br>
-static void submit_packet_vi(struct kernel_queue *kq) -{<br>
-       *kq->wptr_kernel = kq->pending_wptr;<br>
-       write_kernel_doorbell(kq->queue->properties.doorbell_ptr,<br>
-                               kq->pending_wptr);<br>
-}<br>
-<br>
 unsigned int pm_build_pm4_header(unsigned int opcode, size_t packet_size)  {<br>
         union PM4_MES_TYPE_3_HEADER header;<br>
--<br>
2.17.1<br>
<br>
_______________________________________________<br>
amd-gfx mailing list<br>
amd-gfx@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
_______________________________________________<br>
amd-gfx mailing list<br>
amd-gfx@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></div>
</span></font></div>
</body>
</html>